Author: Marco Elver Date: 2026-10-20T12:06:03+02:00 New Revision: 51ec77936fb5423bb713102a8aa35759fa83e116
URL: https://github.com/llvm/llvm-project/commit/51ec77936fb5423bb713102a8aa35759fa83e116 DIFF: https://github.com/llvm/llvm-project/commit/51ec77936fb5423bb713102a8aa35759fa83e116.diff LOG: Revert "[clang][ThreadSafety] Do not read a C field's address as a pointer to member (#231604) (#233614) This reverts commit 217e229a1f32785fccdd3b3f51429f0d309dea45. It causes false positives in code that previously compiled: - https://github.com/ClangBuiltLinux/linux/issues/2187 - https://github.com/llvm/llvm-project/pull/231604#issuecomment-6278791594 In general the old behaviour still should be fixed, but let's give us some time to work out the corner cases. Added: Modified: clang/lib/Analysis/ThreadSafetyCommon.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/test/Sema/warn-thread-safety-analysis.c clang/test/SemaCXX/warn-thread-safety-analysis.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp index fce8141446e876..d616e17c9aaedf 100644 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp @@ -628,12 +628,15 @@ til::SExpr *SExprBuilder::translateUnaryOperator(const UnaryOperator *UO, case UO_AddrOf: if (CapabilityExprMode) { - // A pointer-to-member expression, &MyClass::mu_, is read as a wildcard. - if (UO->getType()->isMemberPointerType()) - if (const auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr())) { + // interpret &Graph::mu_ as an existential. + if (const auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr())) { + if (DRE->getDecl()->isCXXInstanceMember()) { + // This is a pointer-to-member expression, e.g. &MyClass::mu_. + // We interpret this syntax specially, as a wildcard. auto *W = new (Arena) til::Wildcard(); return new (Arena) til::Project(W, DRE->getDecl()); } + } } // otherwise, & is a no-op return translate(UO->getSubExpr(), Ctx); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 6205c705101592..33b680c4d93d4c 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -395,13 +395,13 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, QualType ArgTy = ArgExp->getType(); - // A pointer to member expression of the form &MyClass::mu is treated - // specially -- we need to look at the type of the member. The analysis - // reads it as that member of any object. + // A pointer to member expression of the form &MyClass::mu is treated + // specially -- we need to look at the type of the member. if (const auto *UOp = dyn_cast<UnaryOperator>(ArgExp)) if (UOp->getOpcode() == UO_AddrOf) - if (const auto *MPT = ArgTy->getAs<MemberPointerType>()) - ArgTy = MPT->getPointeeType(); + if (const auto *DRE = dyn_cast<DeclRefExpr>(UOp->getSubExpr())) + if (DRE->getDecl()->isCXXInstanceMember()) + ArgTy = DRE->getDecl()->getType(); // First see if we can just cast to record type, or pointer to record type. const auto *RD = getRecordDecl(ArgTy); diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 369852faa84944..0c02e96470cb21 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -315,34 +315,8 @@ void test_fp_ops(struct FPOps *ops) { } void test_fp_ops_fail(struct FPOps *ops) { - ops->a = 42; // expected-warning {{writing variable 'a' requires holding mutex 'ops->mu' exclusively}} - ops->requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex 'ops->mu' exclusively}} -} - -// '&mu' names the mutex of the same object, as 'mu' does; C has no -// pointer-to-member syntax, so it is not the wildcard '&FPOps::mu' is in C++. -void test_fp_ops_other(struct FPOps *ops, struct FPOps *other) { - other->lock(); - ops->a = 42; // expected-warning {{writing variable 'a' requires holding mutex 'ops->mu' exclusively}} \ - // expected-note {{found near match 'other->mu'}} - ops->requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex 'ops->mu' exclusively}} \ - // expected-note {{found near match 'other->mu'}} - other->unlock(); -} - -// The object may be reached without a pointer, and the field may be called -// through an explicit dereference. -void test_fp_ops_value(struct FPOps foo, struct FPOps *ops) { - foo.lock(); - foo.a = 42; - (*ops->lock)(); - ops->a = 42; - (*foo.unlock)(); - foo.a = 42; // expected-warning {{writing variable 'a' requires holding mutex 'foo.mu' exclusively}} \ - // expected-note {{found near match 'ops->mu'}} - foo.requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex 'foo.mu' exclusively}} \ - // expected-note {{found near match 'ops->mu'}} - ops->unlock(); + ops->a = 42; // expected-warning {{writing variable 'a' requires holding mutex '&FPOps::mu' exclusively}} + ops->requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex '&FPOps::mu' exclusively}} } // Function pointer parameters. The attributes constrain the function reached diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index efd132750bb202..d024f324e524dc 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -8482,24 +8482,4 @@ struct GuardedOpsSelf { } }; -// '&mu' is '&this->mu', the mutex of the called object, whereas the pointer -// to member '&GuardedOpsAddrOf::mu' is the 'mu' of any object. -struct GuardedOpsAddrOf { - Mutex mu; - int data GUARDED_BY(&mu); - int any GUARDED_BY(&GuardedOpsAddrOf::mu); - void (*lock)() EXCLUSIVE_LOCK_FUNCTION(&mu); - void (*unlock)() UNLOCK_FUNCTION(&mu); -}; - -void testMemberAddrOf(GuardedOpsAddrOf *g, GuardedOpsAddrOf *h) { - h->lock(); - h->data = 1; - g->any = 1; - g->data = 1; // expected-warning {{writing variable 'data' requires holding mutex 'g->mu' exclusively}} \ - // expected-note {{found near match 'h->mu'}} - h->unlock(); - g->any = 1; // expected-warning {{writing variable 'any' requires holding mutex '&FunctionPointers::GuardedOpsAddrOf::mu' exclusively}} -} - } // namespace FunctionPointers _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
