forked from OSchip/llvm-project
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.
The previous attempt in 9dcc82f34e
was causing false positives because
I wrongly assumed that LiteralPtrs were always globals, which they are
not. This should be fixed now.
[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:
parent
1ff313f098
commit
5250a03a99
|
@ -1266,13 +1266,24 @@ ClassifyDiagnostic(const AttrTy *A) {
|
||||||
}
|
}
|
||||||
|
|
||||||
bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
|
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 (const auto *LP = dyn_cast<til::LiteralPtr>(SExp)) {
|
||||||
|
const ValueDecl *VD = LP->clangDecl();
|
||||||
|
return VD->isDefinedOutsideFunctionOrMethod();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Members are in scope from methods of the same class.
|
||||||
|
if (const auto *P = dyn_cast<til::Project>(SExp)) {
|
||||||
|
if (!CurrentMethod)
|
||||||
return false;
|
return false;
|
||||||
if (const auto *P = dyn_cast_or_null<til::Project>(CapE.sexpr())) {
|
|
||||||
const auto *VD = P->clangDecl();
|
const auto *VD = P->clangDecl();
|
||||||
if (VD)
|
if (VD)
|
||||||
return VD->getDeclContext() == CurrentMethod->getDeclContext();
|
return VD->getDeclContext() == CurrentMethod->getDeclContext();
|
||||||
}
|
}
|
||||||
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -5036,7 +5036,8 @@ void spawn_fake_flight_control_thread(void) {
|
||||||
}
|
}
|
||||||
|
|
||||||
extern const char *deque_log_msg(void) __attribute__((requires_capability(Logger)));
|
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;
|
const char *msg;
|
||||||
|
|
||||||
while ((msg = deque_log_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);
|
acquire(Logger);
|
||||||
logger_entry();
|
logger_entry();
|
||||||
release(Logger);
|
release(Logger);
|
||||||
}
|
}
|
||||||
|
|
||||||
int main(void) {
|
int main(void) __attribute__((requires_capability(!FlightControl))) {
|
||||||
spawn_fake_flight_control_thread();
|
spawn_fake_flight_control_thread();
|
||||||
spawn_fake_logger_thread();
|
spawn_fake_logger_thread();
|
||||||
|
|
||||||
|
|
|
@ -21,6 +21,12 @@ class LOCKABLE Mutex {
|
||||||
void AssertReaderHeld() ASSERT_SHARED_LOCK();
|
void AssertReaderHeld() ASSERT_SHARED_LOCK();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
class SCOPED_LOCKABLE MutexLock {
|
||||||
|
public:
|
||||||
|
MutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
|
||||||
|
MutexLock(Mutex *mu, bool adopt) EXCLUSIVE_LOCKS_REQUIRED(mu);
|
||||||
|
~MutexLock() UNLOCK_FUNCTION();
|
||||||
|
};
|
||||||
|
|
||||||
namespace SimpleTest {
|
namespace SimpleTest {
|
||||||
|
|
||||||
|
@ -77,10 +83,43 @@ public:
|
||||||
mu.Unlock();
|
mu.Unlock();
|
||||||
baz(); // no warning -- !mu in set.
|
baz(); // no warning -- !mu in set.
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void test4() {
|
||||||
|
MutexLock lock(&mu); // expected-warning {{acquiring mutex 'mu' requires negative capability '!mu'}}
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
} // end namespace SimpleTest
|
} // 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 {
|
namespace DoubleAttribute {
|
||||||
|
|
||||||
struct Foo {
|
struct Foo {
|
||||||
|
|
Loading…
Reference in New Issue