[analyzer] Don't escape local static memregions on bind
authorBalazs Benics <benicsbalazs@gmail.com>
Thu, 12 Jan 2023 09:42:57 +0000 (10:42 +0100)
committerBalazs Benics <benicsbalazs@gmail.com>
Thu, 12 Jan 2023 09:42:57 +0000 (10:42 +0100)
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

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/test/Analysis/malloc-static-storage.cpp [new file with mode: 0644]

index 87a95d46f298732a8f3d8668c2c7642c30bf162c..0b8800294c453d2d0c0a79e6647ee4ac27729baa 100644 (file)
@@ -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 (file)
index 0000000..4eeabab
--- /dev/null
@@ -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);
+}