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

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, Szelethus.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139534

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/malloc-static-storage.cpp


Index: clang/test/Analysis/malloc-static-storage.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/malloc-static-storage.cpp
@@ -0,0 +1,58 @@
+// 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
+}
+
+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.
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3461,7 +3461,8 @@
   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;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139534.480889.patch
Type: text/x-patch
Size: 2225 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221207/b2e62a21/attachment.bin>