llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/150240.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+8-6) - (modified) clang/test/Analysis/malloc.c (+14) ``````````diff 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}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/150240 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
