Thread safety analysis: Consider global variables in scope
Instead of just mutex members we also consider mutex globals. Unsurprisingly they are always in scope. Now the paper [1] 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. [1] https://static.googleusercontent.com/media/research.google.com/en/us/pubs/archive/42958.pdf Fixes PR46354. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D84604
This commit is contained in:
@@ -1266,13 +1266,22 @@ ClassifyDiagnostic(const AttrTy *A) {
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -5036,7 +5036,8 @@ void spawn_fake_flight_control_thread(void) {
|
||||
}
|
||||
|
||||
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 logger_entry(void) __attribute__((requires_capability(Logger))) {
|
||||
}
|
||||
}
|
||||
|
||||
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();
|
||||
|
||||
|
||||
@@ -81,6 +81,35 @@ public:
|
||||
|
||||
} // end namespace SimpleTest
|
||||
|
||||
Mutex globalMutex;
|
||||
|
||||
namespace ScopeTest {
|
||||
|
||||
void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
|
||||
void fq() EXCLUSIVE_LOCKS_REQUIRED(!::globalMutex);
|
||||
|
||||
namespace ns {
|
||||
Mutex globalMutex;
|
||||
void f() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex);
|
||||
void fq() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex);
|
||||
}
|
||||
|
||||
void testGlobals() EXCLUSIVE_LOCKS_REQUIRED(!ns::globalMutex) {
|
||||
f(); // expected-warning {{calling function 'f' requires negative capability '!globalMutex'}}
|
||||
fq(); // expected-warning {{calling function 'fq' requires negative capability '!globalMutex'}}
|
||||
ns::f();
|
||||
ns::fq();
|
||||
}
|
||||
|
||||
void testNamespaceGlobals() EXCLUSIVE_LOCKS_REQUIRED(!globalMutex) {
|
||||
f();
|
||||
fq();
|
||||
ns::f(); // expected-warning {{calling function 'f' requires negative capability '!globalMutex'}}
|
||||
ns::fq(); // expected-warning {{calling function 'fq' requires negative capability '!globalMutex'}}
|
||||
}
|
||||
|
||||
} // end namespace ScopeTest
|
||||
|
||||
namespace DoubleAttribute {
|
||||
|
||||
struct Foo {
|
||||
|
||||
Reference in New Issue
Block a user