aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Instead of just mutex members we also consider mutex globals. Unsurprisingly they are always in scope. Now the paper says that The scope of a class member is assumed to be its enclosing class, while the scope of a global variable is the translation unit in which it is defined. But I don't think we should limit this to TUs where a definition is available - a declaration is enough to acquire the mutex, and if a mutex is really limited in scope to a translation unit, it should probably be only declared there. Fixes PR46354. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D84604 Files: clang/lib/Analysis/ThreadSafety.cpp clang/test/SemaCXX/warn-thread-safety-analysis.cpp clang/test/SemaCXX/warn-thread-safety-negative.cpp Index: clang/test/SemaCXX/warn-thread-safety-negative.cpp =================================================================== --- clang/test/SemaCXX/warn-thread-safety-negative.cpp +++ clang/test/SemaCXX/warn-thread-safety-negative.cpp @@ -81,6 +81,23 @@ } // end namespace SimpleTest +namespace ScopeTest { + +Mutex globalMutex; +void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex); + +namespace ns { + Mutex namespaceMutex; + void f() EXCLUSIVE_LOCKS_REQUIRED(!namespaceMutex); +} + +void testGlobals() { + f(); // expected-warning {{calling function 'f' requires holding negative capability '!globalMutex'}} + ns::f(); // expected-warning {{calling function 'f' requires holding negative capability '!namespaceMutex'}} +} + +} // end namespace ScopeTest + namespace DoubleAttribute { struct Foo { Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp =================================================================== --- clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -5036,7 +5036,8 @@ } extern const char *deque_log_msg(void) __attribute__((requires_capability(Logger))); -void logger_entry(void) __attribute__((requires_capability(Logger))) { +void logger_entry(void) __attribute__((requires_capability(Logger))) + __attribute__((requires_capability(!FlightControl))) { const char *msg; while ((msg = deque_log_msg())) { @@ -5044,13 +5045,13 @@ } } -void spawn_fake_logger_thread(void) { +void spawn_fake_logger_thread(void) __attribute__((requires_capability(!FlightControl))) { acquire(Logger); logger_entry(); release(Logger); } -int main(void) { +int main(void) __attribute__((requires_capability(!FlightControl))) { spawn_fake_flight_control_thread(); spawn_fake_logger_thread(); Index: clang/lib/Analysis/ThreadSafety.cpp =================================================================== --- clang/lib/Analysis/ThreadSafety.cpp +++ clang/lib/Analysis/ThreadSafety.cpp @@ -1266,13 +1266,22 @@ } bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) { - if (!CurrentMethod) + const threadSafety::til::SExpr *SExp = CapE.sexpr(); + assert(SExp && "Null expressions should be ignored"); + + // Global variables are always in scope. + if (isa<til::LiteralPtr>(SExp)) + return true; + + // Members are in scope from methods of the same class. + if (const auto *P = dyn_cast<til::Project>(SExp)) { + if (!CurrentMethod) return false; - if (const auto *P = dyn_cast_or_null<til::Project>(CapE.sexpr())) { const auto *VD = P->clangDecl(); if (VD) return VD->getDeclContext() == CurrentMethod->getDeclContext(); } + return false; }
Index: clang/test/SemaCXX/warn-thread-safety-negative.cpp =================================================================== --- clang/test/SemaCXX/warn-thread-safety-negative.cpp +++ clang/test/SemaCXX/warn-thread-safety-negative.cpp @@ -81,6 +81,23 @@ } // end namespace SimpleTest +namespace ScopeTest { + +Mutex globalMutex; +void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex); + +namespace ns { + Mutex namespaceMutex; + void f() EXCLUSIVE_LOCKS_REQUIRED(!namespaceMutex); +} + +void testGlobals() { + f(); // expected-warning {{calling function 'f' requires holding negative capability '!globalMutex'}} + ns::f(); // expected-warning {{calling function 'f' requires holding negative capability '!namespaceMutex'}} +} + +} // end namespace ScopeTest + namespace DoubleAttribute { struct Foo { Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp =================================================================== --- clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -5036,7 +5036,8 @@ } extern const char *deque_log_msg(void) __attribute__((requires_capability(Logger))); -void logger_entry(void) __attribute__((requires_capability(Logger))) { +void logger_entry(void) __attribute__((requires_capability(Logger))) + __attribute__((requires_capability(!FlightControl))) { const char *msg; while ((msg = deque_log_msg())) { @@ -5044,13 +5045,13 @@ } } -void spawn_fake_logger_thread(void) { +void spawn_fake_logger_thread(void) __attribute__((requires_capability(!FlightControl))) { acquire(Logger); logger_entry(); release(Logger); } -int main(void) { +int main(void) __attribute__((requires_capability(!FlightControl))) { spawn_fake_flight_control_thread(); spawn_fake_logger_thread(); Index: clang/lib/Analysis/ThreadSafety.cpp =================================================================== --- clang/lib/Analysis/ThreadSafety.cpp +++ clang/lib/Analysis/ThreadSafety.cpp @@ -1266,13 +1266,22 @@ } bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) { - if (!CurrentMethod) + const threadSafety::til::SExpr *SExp = CapE.sexpr(); + assert(SExp && "Null expressions should be ignored"); + + // Global variables are always in scope. + if (isa<til::LiteralPtr>(SExp)) + return true; + + // Members are in scope from methods of the same class. + if (const auto *P = dyn_cast<til::Project>(SExp)) { + if (!CurrentMethod) return false; - if (const auto *P = dyn_cast_or_null<til::Project>(CapE.sexpr())) { const auto *VD = P->clangDecl(); if (VD) return VD->getDeclContext() == CurrentMethod->getDeclContext(); } + return false; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits