[clang] 025541d - [analyzer] Relax assertion in BugReporterVisitors.cpp isInitializationOfVar (#125044)

[via cfe-commits]

Author: Balazs Benics
Date: 2025-01-30T12:48:09+01:00
New Revision: 025541ddedd23e39d9394ea8a1e41a64dfbe4e94

URL: https://github.com/llvm/llvm-project/commit/025541ddedd23e39d9394ea8a1e41a64dfbe4e94
DIFF: https://github.com/llvm/llvm-project/commit/025541ddedd23e39d9394ea8a1e41a64dfbe4e94.diff

LOG: [analyzer] Relax assertion in BugReporterVisitors.cpp isInitializationOfVar (#125044)

If we see a variable declaration (aka. DeclStmt), and the VarRegion it
declared doesn't have Stack memspace, we assumed that it must be a local
static variable.
However, the declared variable may be an extern declaration of a global.

In this patch, let's admit that local extern declarations are a thing.

For the sake of completeness, I also added one more test for
thread_locals - which are implicitly considered statics btw. (the
`isStaticLocal()` correctly also considers thread locals as local
statics).

Fixes #124975

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    clang/test/Analysis/null-deref-path-notes.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index a9b4dbb39b5bd62..a6142063895dbd1 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1198,7 +1198,10 @@ static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
     // If we ever directly evaluate global DeclStmts, this assertion will be
     // invalid, but this still seems preferable to silently accepting an
     // initialization that may be for a path-sensitive variable.
-    assert(VR->getDecl()->isStaticLocal() && "non-static stackless VarRegion");
+    [[maybe_unused]] bool IsLocalStaticOrLocalExtern =
+        VR->getDecl()->isStaticLocal() || VR->getDecl()->isLocalExternDecl();
+    assert(IsLocalStaticOrLocalExtern &&
+           "Declared a variable on the stack without Stack memspace?");
     return true;
   }
 

diff  --git a/clang/test/Analysis/null-deref-path-notes.cpp b/clang/test/Analysis/null-deref-path-notes.cpp
index c7b0619e297b3c5..a37bbfe41a2c705 100644
--- a/clang/test/Analysis/null-deref-path-notes.cpp
+++ b/clang/test/Analysis/null-deref-path-notes.cpp
@@ -23,3 +23,38 @@ void c::f(B &g, int &i) {
   f(h, b); // expected-note{{Calling 'c::f'}}
 }
 }
+
+namespace GH124975 {
+void no_crash_in_br_visitors(int *p) {
+  if (p) {}
+  // expected-note at -1 {{Assuming 'p' is null}}
+  // expected-note at -2 {{Taking false branch}}
+
+  extern bool ExternLocalCoin;
+  // expected-note at +2 {{Assuming 'ExternLocalCoin' is false}}
+  // expected-note at +1 {{Taking false branch}}
+  if (ExternLocalCoin)
+    return;
+
+  *p = 4;
+  // expected-warning at -1 {{Dereference of null pointer (loaded from variable 'p')}}
+  // expected-note at -2    {{Dereference of null pointer (loaded from variable 'p')}}
+}
+
+// Thread local variables are implicitly static, so let's test them too.
+void thread_local_alternative(int *p) {
+  if (p) {}
+  // expected-note at -1 {{Assuming 'p' is null}}
+  // expected-note at -2 {{Taking false branch}}
+
+  thread_local bool ThreadLocalCoin;
+  // expected-note at +2 {{'ThreadLocalCoin' is false}}
+  // expected-note at +1 {{Taking false branch}}
+  if (ThreadLocalCoin)
+    return;
+
+  *p = 4;
+  // expected-warning at -1 {{Dereference of null pointer (loaded from variable 'p')}}
+  // expected-note at -2    {{Dereference of null pointer (loaded from variable 'p')}}
+}
+} // namespace GH124975