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@-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
