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;
}
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
