[clang] 3c8c0d4 - Thread Safety Analysis: Handle address-of followed by dereference

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2025-02-26T16:34:33+01:00
New Revision: 3c8c0d4d8d9bbc160d160e683f7a74fd28574dc6

URL: https://github.com/llvm/llvm-project/commit/3c8c0d4d8d9bbc160d160e683f7a74fd28574dc6
DIFF: https://github.com/llvm/llvm-project/commit/3c8c0d4d8d9bbc160d160e683f7a74fd28574dc6.diff

LOG: Thread Safety Analysis: Handle address-of followed by dereference

Correctly analyze expressions where the address of a guarded variable is
taken and immediately dereferenced, such as (*(type-specifier *)&x).
Previously, such patterns would result in false negatives.

Pull Request: https://github.com/llvm/llvm-project/pull/127396

Added: 
    

Modified: 
    clang/lib/Analysis/ThreadSafety.cpp
    clang/test/Sema/warn-thread-safety-analysis.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index bfaf1a0e7c7ff..1bad4eac23c68 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1765,6 +1765,8 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
 void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
                                          AccessKind AK,
                                          ProtectedOperationKind POK) {
+  // Strip off paren- and cast-expressions, checking if we encounter any other
+  // operator that should be delegated to checkAccess() instead.
   while (true) {
     if (const auto *PE = dyn_cast<ParenExpr>(Exp)) {
       Exp = PE->getSubExpr();
@@ -1783,6 +1785,15 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
     break;
   }
 
+  if (const auto *UO = dyn_cast<UnaryOperator>(Exp)) {
+    if (UO->getOpcode() == UO_AddrOf) {
+      // Pointer access via pointer taken of variable, so the dereferenced
+      // variable is not actually a pointer.
+      checkAccess(FSet, UO->getSubExpr(), AK, POK);
+      return;
+    }
+  }
+
   // Pass by reference warnings are under a 
diff erent flag.
   ProtectedOperationKind PtPOK = POK_VarDereference;
   if (POK == POK_PassByRef) PtPOK = POK_PtPassByRef;

diff  --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 73b4e4798f644..46495ad4889b4 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -24,6 +24,9 @@
   __attribute__ ((shared_locks_required(__VA_ARGS__)))
 #define NO_THREAD_SAFETY_ANALYSIS  __attribute__ ((no_thread_safety_analysis))
 
+#define __READ_ONCE(x)        (*(const volatile __typeof__(x) *)&(x))
+#define __WRITE_ONCE(x, val)  do { *(volatile __typeof__(x) *)&(x) = (val); } while (0)
+
 // Define the mutex struct.
 // Simplified only for test purpose.
 struct LOCKABLE Mutex {};
@@ -142,9 +145,25 @@ int main(void) {
   (void)(get_value(b_) == 1);
   mutex_unlock(foo_.mu_);
 
+  a_ = 0; // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
+  __WRITE_ONCE(a_, 0); // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
+  (void)(a_ == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
+  (void)(__READ_ONCE(a_) == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
+  *b_ = 0; // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
+  __WRITE_ONCE(*b_, 0); // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
+  (void)(*b_ == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
+  (void)(__READ_ONCE(*b_) == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
   c_ = 0; // expected-warning{{writing variable 'c_' requires holding any mutex exclusively}}
   (void)(*d_ == 0); // expected-warning{{reading the value pointed to by 'd_' requires holding any mutex}}
   mutex_exclusive_lock(foo_.mu_);
+  a_ = 0;
+  __WRITE_ONCE(a_, 0);
+  (void)(a_ == 0);
+  (void)(__READ_ONCE(a_) == 0);
+  *b_ = 0;
+  __WRITE_ONCE(*b_, 0);
+  (void)(*b_ == 0);
+  (void)(__READ_ONCE(*b_) == 0);
   c_ = 1;
   (void)(*d_ == 1);
   mutex_unlock(foo_.mu_);