[clang] 8427885 - Temporairly revert "Thread safety analysis: Consider global variables in scope" & followup

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2020-09-09T12:15:56+03:00
New Revision: 8427885e27813c457dccb011f65e8ded74444e31

URL: https://github.com/llvm/llvm-project/commit/8427885e27813c457dccb011f65e8ded74444e31
DIFF: https://github.com/llvm/llvm-project/commit/8427885e27813c457dccb011f65e8ded74444e31.diff

LOG: 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 9dcc82f34ea9b623d82d2577b93aaf67d36dabd2.
This reverts commit b2ce79ef66157dd752e3864ece57915e23a73f5d.

Added: 
    

Modified: 
    clang/lib/Analysis/ThreadSafety.cpp
    clang/lib/Analysis/ThreadSafetyCommon.cpp
    clang/test/SemaCXX/warn-thread-safety-analysis.cpp
    clang/test/SemaCXX/warn-thread-safety-negative.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 5b97265a6d8a..64e0da9e64b1 100644
--- 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
index aee918576007..1b8c55e56d47 100644
--- 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
index d1520b1decbd..91bd15def577 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5036,8 +5036,7 @@ 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())) {
@@ -5045,13 +5044,13 @@ void logger_entry(void) __attribute__((requires_capability(Logger)))
   }
 }
 
-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
index 68e30f4a3225..456fe16e6574 100644
--- 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 {