Author: Donát Nagy Date: 2025-07-24T16:18:37+02:00 New Revision: deede2b2db262a932f07a386e59c2ca0b1a798d1
URL: https://github.com/llvm/llvm-project/commit/deede2b2db262a932f07a386e59c2ca0b1a798d1 DIFF: https://github.com/llvm/llvm-project/commit/deede2b2db262a932f07a386e59c2ca0b1a798d1.diff LOG: [analyzer] Eliminate unique release point assertion (#150240) 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 and modify the way the state of the symbol is changed.) Fixes #149754 Added: Modified: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/malloc.c Removed: ################################################################################ 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..a9828cfbc1934 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1954,9 +1954,23 @@ 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); } + +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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
