[clang] 840edd8 - [analyzer] Don't escape local static memregions on bind

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2023-01-12T10:42:57+01:00
New Revision: 840edd8ab2620a52e9acbef7de037c9f465dfce7

URL: https://github.com/llvm/llvm-project/commit/840edd8ab2620a52e9acbef7de037c9f465dfce7
DIFF: https://github.com/llvm/llvm-project/commit/840edd8ab2620a52e9acbef7de037c9f465dfce7.diff

LOG: [analyzer] Don't escape local static memregions on bind

When the engine processes a store to a variable, it will eventually call
`ExprEngine::processPointerEscapedOnBind()`. This function is supposed to
invalidate (put the given locations to an escape list) the locations
which we cannot reason about.

Unfortunately, local static variables are also put into this list.

This patch relaxes the guard condition, so that beyond stack variables,
static local variables are also ignored.

Differential Revision: https://reviews.llvm.org/D139534

Added: 
    clang/test/Analysis/malloc-static-storage.cpp

Modified: 
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 87a95d46f2987..0b8800294c453 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3461,7 +3461,8 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(
   for (const std::pair<SVal, SVal> &LocAndVal : LocAndVals) {
     // Cases (1) and (2).
     const MemRegion *MR = LocAndVal.first.getAsRegion();
-    if (!MR || !MR->hasStackStorage()) {
+    if (!MR ||
+        !isa<StackSpaceRegion, StaticGlobalSpaceRegion>(MR->getMemorySpace())) {
       Escaped.push_back(LocAndVal.second);
       continue;
     }

diff  --git a/clang/test/Analysis/malloc-static-storage.cpp b/clang/test/Analysis/malloc-static-storage.cpp
new file mode 100644
index 0000000000000..4eeabab41209b
--- /dev/null
+++ b/clang/test/Analysis/malloc-static-storage.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+void* malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void free(void * ptr);
+
+void escape(void *);
+void next_statement();
+
+void conditional_malloc(bool coin) {
+  static int *p;
+
+  if (coin) {
+    p = (int *)malloc(sizeof(int));
+  }
+  p = 0; // Pointee of 'p' dies, which is recognized at the next statement.
+  next_statement(); // expected-warning {{Potential memory leak}}
+}
+
+void malloc_twice() {
+  static int *p;
+  p = (int *)malloc(sizeof(int));
+  next_statement();
+  p = (int *)malloc(sizeof(int));
+  next_statement(); // expected-warning {{Potential memory leak}}
+  p = 0;
+  next_statement(); // expected-warning {{Potential memory leak}}
+}
+
+void malloc_escape() {
+  static int *p;
+  p = (int *)malloc(sizeof(int));
+  escape(p); // no-leak
+  p = 0; // no-leak
+}
+
+void free_whatever_escaped();
+void malloc_escape_reversed() {
+  static int *p;
+  escape(&p);
+  p = (int *)malloc(sizeof(int));
+  free_whatever_escaped();
+  p = 0; // FIXME: We should not report a leak here.
+  next_statement(); // expected-warning {{Potential memory leak}}
+}
+
+int *malloc_return_static() {
+  static int *p = (int *)malloc(sizeof(int));
+  return p; // no-leak
+}
+
+int malloc_unreachable(int rng) {
+  // 'p' does not escape and never freed :(
+  static int *p;
+
+  // For the second invocation of this function, we leak the previous pointer.
+  // FIXME: We should catch this at some point.
+  p = (int *)malloc(sizeof(int));
+  *p = 0;
+
+  if (rng > 0)
+    *p = rng;
+
+  return *p; // FIXME: We just leaked 'p'. We should warn about this.
+}
+
+void malloc_cond(bool cond) {
+  static int *p;
+  if (cond) {
+    p = (int*)malloc(sizeof(int));
+    free_whatever_escaped();
+    p = 0; // FIXME: We should not report a leak here.
+    next_statement(); // expected-warning {{Potential memory leak}}
+  }
+  escape(&p);
+}