https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/150240
From 65542ca9e45c5f29139bf1ad5e8925c3337439c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Wed, 23 Jul 2025 15:20:34 +0200
Subject: [PATCH 1/2] [analyzer] Eliminate unique release point assertion
MallocChecker.cpp has a complex heuristic that supresses reports where
the memory release happens during the release of a reference-counted
object (to suppress a significant amount of false positives).
Previously this logic asserted that there is at most one release point
corresponding to a symbol, but it turns out that there is a rare corner
case where the symbol can be released, forgotten and then released
again. This commit removes that assertion to avoid the crash. (As this
issue just affects a bug suppression heuristic, I didn't want to dig
deeper in the state changes.)
Fixes #149754
---
.../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 14 ++++++++------
clang/test/Analysis/malloc.c | 14 ++++++++++++++
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 68efdbaec341b..a7704da82fcc2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -3730,13 +3730,15 @@ PathDiagnosticPieceRef
MallocBugVisitor::VisitNode(const ExplodedNode *N,
return nullptr;
}
- // Save the first destructor/function as release point.
- assert(!ReleaseFunctionLC && "There should be only one release point");
+ // Record the stack frame that is _responsible_ for this memory release
+ // event. This will be used by the false positive suppression
heuristics
+ // that recognize the release points of reference-counted objects.
+ //
+ // Usually (e.g. in C) we say that the _responsible_ stack frame is the
+ // current innermost stack frame:
ReleaseFunctionLC = CurrentLC->getStackFrame();
-
- // See if we're releasing memory while inlining a destructor that
- // decrement reference counters (or one of its callees).
- // This turns on various common false positive suppressions.
+ // ...but if the stack contains a destructor call, then we say that the
+ // outermost destructor stack frame is the _responsible_ one:
for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) {
if (const auto *DD = dyn_cast<CXXDestructorDecl>(LC->getDecl())) {
if (isReferenceCountingPointerDestructor(DD)) {
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 27a04ff873521..877b187e65997 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1960,3 +1960,17 @@ void testExtent(void) {
// expected-warning-re@-1 {{{{^conj_\$[[:digit:]]+{int, LC1, S[[:digit:]]+,
#1}}}}}}
free(p);
}
+
+void gh149754(void *p) {
+ // This testcase demonstrates an unusual situation where a certain symbol
+ // (the value of `p`) is released (more precisely, transitions from
+ // untracked state to Released state) twice within the same bug path because
+ // the `EvalAssume` callback resets it to untracked state after the first
+ // time when it is released. This caused the failure of an assertion, which
+ // was since then removed for the codebase.
+ if (!realloc(p, 8)) {
+ realloc(p, 8);
+ free(p); // expected-warning {{Attempt to free released memory}}
+ }
+ // expected-warning@+1 {{Potential memory leak}}
+}
From fcdeccff48da9751bb14de988d8b38fe708ab3e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Thu, 24 Jul 2025 14:35:24 +0200
Subject: [PATCH 2/2] Don't check location context id value in test
The ID of a location context is a completely arbitrary number
(`LocationContextManager` issues consecutive numbers as the IDs of the
location contexts that it constructs) which is irrelevant for that
particular test, so use a wildcard for matching it (instead of a
concrete number).
This check made the test break on a Windows buildbot, which I cannot
investigate (because I don't have access to Windows), so I'm
sidestepping the issue with this commit.
---
clang/test/Analysis/malloc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 877b187e65997..a9828cfbc1934 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1954,10 +1954,10 @@ int conjure(void);
void testExtent(void) {
int x = conjure();
clang_analyzer_dump(x);
- // expected-warning-re@-1 {{{{^conj_\$[[:digit:]]+{int, LC1, S[[:digit:]]+,
#1}}}}}}
+ // expected-warning-re@-1 {{{{^conj_\$[[:digit:]]+{int, LC[[:digit:]]+,
S[[:digit:]]+, #1}}}}}}
int *p = (int *)malloc(x);
clang_analyzer_dumpExtent(p);
- // expected-warning-re@-1 {{{{^conj_\$[[:digit:]]+{int, LC1, S[[:digit:]]+,
#1}}}}}}
+ // expected-warning-re@-1 {{{{^conj_\$[[:digit:]]+{int, LC[[:digit:]]+,
S[[:digit:]]+, #1}}}}}}
free(p);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
