Temporairly revert "Thread safety analysis: Consider global variables…

… in scope" & followup This appears to cause false-positives because it started to warn on local non-global variables. Repro posted to https://reviews.llvm.org/D84604#2262745 This reverts commit 9dcc82f. This reverts commit b2ce79e.
jaebaek · Sep 9, 2020 · 8427885 · 8427885
1 parent feb0b9c
commit 8427885
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 47 deletions.
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1266,21 +1266,13 @@ ClassifyDiagnostic(const AttrTy *A) {
 }
 
 bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
-  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)
+  if (!CurrentMethod)
       return false;
-    const ValueDecl *VD = P->clangDecl();
-    return VD->getDeclContext() == CurrentMethod->getDeclContext();
+  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;
 }
 

diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -274,7 +274,7 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
   const auto *VD = cast<ValueDecl>(DRE->getDecl()->getCanonicalDecl());
 
   // Function parameters require substitution and/or renaming.
-  if (const auto *PV = dyn_cast<ParmVarDecl>(VD)) {
+  if (const auto *PV = dyn_cast_or_null<ParmVarDecl>(VD)) {
     unsigned I = PV->getFunctionScopeIndex();
     const DeclContext *D = PV->getDeclContext();
     if (Ctx && Ctx->FunArgs) {

diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5036,22 +5036,21 @@ 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)))
-                        __attribute__((requires_capability(!FlightControl))) {
+void logger_entry(void) __attribute__((requires_capability(Logger))) {
   const char *msg;
 
   while ((msg = deque_log_msg())) {
     dispatch_log(msg);
   }
 }
 
-void spawn_fake_logger_thread(void) __attribute__((requires_capability(!FlightControl))) {
+void spawn_fake_logger_thread(void) {
   acquire(Logger);
   logger_entry();
   release(Logger);
 }
 
-int main(void) __attribute__((requires_capability(!FlightControl))) {
+int main(void) {
   spawn_fake_flight_control_thread();
   spawn_fake_logger_thread();
 

diff --git a/clang/test/SemaCXX/warn-thread-safety-negative.cpp b/clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -81,35 +81,6 @@ class Foo {
 
 }  // 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 {