https://github.com/steakhal created
https://github.com/llvm/llvm-project/pull/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
>From 797e3582c98b880fb3a55f0a0594bc200d77ef71 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbal...@gmail.com>
Date: Thu, 30 Jan 2025 11:01:13 +0100
Subject: [PATCH] [analyzer] Relax assertion in BugReporterVisitors.cpp
isInitializationOfVar
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).
---
.../Core/BugReporterVisitors.cpp | 5 ++-
clang/test/Analysis/null-deref-path-notes.cpp | 35 +++++++++++++++++++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index a9b4dbb39b5bd6..a6142063895dbd 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 c7b0619e297b3c..a37bbfe41a2c70 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@-1 {{Assuming 'p' is null}}
+ // expected-note@-2 {{Taking false branch}}
+
+ extern bool ExternLocalCoin;
+ // expected-note@+2 {{Assuming 'ExternLocalCoin' is false}}
+ // expected-note@+1 {{Taking false branch}}
+ if (ExternLocalCoin)
+ return;
+
+ *p = 4;
+ // expected-warning@-1 {{Dereference of null pointer (loaded from variable
'p')}}
+ // expected-note@-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@-1 {{Assuming 'p' is null}}
+ // expected-note@-2 {{Taking false branch}}
+
+ thread_local bool ThreadLocalCoin;
+ // expected-note@+2 {{'ThreadLocalCoin' is false}}
+ // expected-note@+1 {{Taking false branch}}
+ if (ThreadLocalCoin)
+ return;
+
+ *p = 4;
+ // expected-warning@-1 {{Dereference of null pointer (loaded from variable
'p')}}
+ // expected-note@-2 {{Dereference of null pointer (loaded from variable
'p')}}
+}
+} // namespace GH124975
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
