https://github.com/voyager-jhk updated https://github.com/llvm/llvm-project/pull/221587
>From bdcca58411d202c007d2ef4fc66279c73995e4e8 Mon Sep 17 00:00:00 2001 From: voyager-jhk <[email protected]> Date: Sun, 6 Sep 2026 23:52:43 +0800 Subject: [PATCH] [clang] Fix NewDeleteLeaks false negative for heap fields Pointers stored in heap fields are incorrectly considered escaped, causing leaks to go unreported. Fixes #214226 --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 73 ++++++++++++++++++- clang/test/Analysis/NewDeleteLeaks.cpp | 51 +++++++++++++ clang/test/Analysis/malloc.c | 15 +++- clang/test/Analysis/pr22954.c | 9 ++- 4 files changed, 140 insertions(+), 8 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 5ee5601a9319a..b083a44cb767a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -272,6 +272,9 @@ MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, REGISTER_SET_WITH_PROGRAMSTATE(ReallocSizeZeroSymbols, SymbolRef) +/// Symbols to suppress for the current PSK_EscapeOnBind. +REGISTER_SET_WITH_PROGRAMSTATE(SuppressEscapeOnBind, SymbolRef) + namespace { /// The state of 'fromPtr' after reallocation is known to have failed. @@ -388,8 +391,8 @@ struct DynMemFrontend : virtual public CheckerFrontend, public BT_PROVIDERS... { class MallocChecker : public CheckerFamily< check::DeadSymbols, check::PointerEscape, check::ConstPointerEscape, - check::PreStmt<ReturnStmt>, check::EndFunction, check::PreCall, - check::PostCall, eval::Call, check::NewAllocator, + check::Bind, check::PreStmt<ReturnStmt>, check::EndFunction, + check::PreCall, check::PostCall, eval::Call, check::NewAllocator, check::PostStmt<BlockExpr>, check::PostObjCMessage, check::Location, eval::Assume> { public: @@ -451,6 +454,8 @@ class MallocChecker bool Assumption) const; void checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const; + void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit, + CheckerContext &C) const; ProgramStateRef checkPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, @@ -1178,6 +1183,33 @@ class EscapeTrackedCallback final : public SymbolVisitor { friend class SymbolVisitor; }; + +/// Collect allocated symbols reachable from a bound value. +class CollectAllocatedOnBindCallback final : public SymbolVisitor { + ProgramStateRef State; + +public: + explicit CollectAllocatedOnBindCallback(ProgramStateRef S) + : State(std::move(S)) {} + ProgramStateRef getState() const { return State; } + + bool VisitSymbol(SymbolRef Sym) override { + if (const RefState *RS = State->get<RegionState>(Sym)) + if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) + State = State->add<SuppressEscapeOnBind>(Sym); + return true; + } +}; + +static bool isAllocatedOrZeroAllocated(const RefState *RS) { + return RS && (RS->isAllocated() || RS->isAllocatedOfSizeZero()); +} + +static ProgramStateRef consumeSuppressEscapeOnBind(ProgramStateRef State) { + if (State->get<SuppressEscapeOnBind>().isEmpty()) + return State; + return State->remove<SuppressEscapeOnBind>(); +} } // end anonymous namespace static bool isStandardNew(const FunctionDecl *FD) { @@ -3893,6 +3925,33 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( return false; } +void MallocChecker::checkBind(SVal Loc, SVal Val, const Stmt *S, + bool AtDeclInit, CheckerContext &C) const { + // Preserve tracking for allocations stored in heap objects that are still + // owned by the analyzer. An escaped parent does not provide this guarantee. + const MemRegion *MR = Loc.getAsRegion(); + if (!MR) + return; + + ProgramStateRef State = C.getState(); + if (!MR->hasMemorySpace<HeapSpaceRegion>(State)) + return; + + const SymbolicRegion *SymBase = MR->getSymbolicBase(); + if (!SymBase) + return; + + const RefState *ParentRS = State->get<RegionState>(SymBase->getSymbol()); + if (!isAllocatedOrZeroAllocated(ParentRS)) + return; + + ProgramStateRef NewState = + State->scanReachableSymbols<CollectAllocatedOnBindCallback>(Val) + .getState(); + if (NewState != State) + C.addTransition(NewState); +} + ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, @@ -3919,6 +3978,9 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, PointerEscapeKind Kind, bool IsConstPointerEscape) const { + // The suppression set only applies to bind escapes. + const bool SuppressOnBind = Kind == PSK_EscapeOnBind; + // If we know that the call does not free memory, or we want to process the // call later, keep tracking the top level arguments. SymbolRef EscapingSymbol = nullptr; @@ -3926,19 +3988,22 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux( !mayFreeAnyEscapedMemoryOrIsModeledExplicitly(Call, State, EscapingSymbol) && !EscapingSymbol) { - return State; + return consumeSuppressEscapeOnBind(State); } for (SymbolRef sym : Escaped) { if (EscapingSymbol && EscapingSymbol != sym) continue; + if (SuppressOnBind && State->contains<SuppressEscapeOnBind>(sym)) + continue; + if (const RefState *RS = State->get<RegionState>(sym)) if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS)) State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } - return State; + return consumeSuppressEscapeOnBind(State); } bool MallocChecker::isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext &C, diff --git a/clang/test/Analysis/NewDeleteLeaks.cpp b/clang/test/Analysis/NewDeleteLeaks.cpp index d9c4b77d1e6fc..1300aba5293eb 100644 --- a/clang/test/Analysis/NewDeleteLeaks.cpp +++ b/clang/test/Analysis/NewDeleteLeaks.cpp @@ -251,3 +251,54 @@ void validate_system_header() { } } // namespace protobuf_leak + +// Regression test for GH#214226. +namespace heap_field_bind { + +struct Owner { + int *member; + Owner() : member(new int(42)) {} // expected-note {{Memory is allocated}} +}; + +void member_leaked_when_owner_deleted() { + Owner *owner = new Owner; // expected-note {{Calling default constructor for 'Owner'}} + // expected-note@-1 {{Returning from default constructor for 'Owner'}} + delete owner; +} // expected-warning {{Potential leak of memory pointed to by field 'member'}} +// expected-note@-1 {{Potential leak of memory pointed to by field 'member'}} + +Owner *member_not_leaked_when_owner_returned() { + Owner *owner = new Owner; + return owner; +} // no-warning + +void member_released_before_owner() { + Owner *owner = new Owner; + delete owner->member; + delete owner; +} // no-warning + +struct OwnerNoCtor { + int *member; +}; + +void leak_assigned_member_when_owner_deleted() { + OwnerNoCtor *owner = new OwnerNoCtor; + owner->member = new int; // expected-note {{Memory is allocated}} + delete owner; +} // expected-warning {{Potential leak of memory pointed to by field 'member'}} +// expected-note@-1 {{Potential leak of memory pointed to by field 'member'}} + +OwnerNoCtor *assigned_member_not_leaked_when_owner_returned() { + OwnerNoCtor *owner = new OwnerNoCtor; + owner->member = new int; + return owner; +} // no-warning + +void escaped_parent_does_not_report_inner(OwnerNoCtor **out) { + OwnerNoCtor *owner = new OwnerNoCtor; + *out = owner; + owner->member = new int; +} // no-warning + +} // namespace heap_field_bind diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 6c3dadfd16021..18a36687a2d7b 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1771,7 +1771,20 @@ void testMallocIntoMalloc(void) { StructWithPtr *s = malloc(sizeof(StructWithPtr)); s->memP = malloc(sizeof(int)); free(s); -} // FIXME: should warn here +} // expected-warning {{Potential leak of memory pointed to by field 'memP'}} + +void testMallocIntoMallocThenFreeMember(void) { + StructWithPtr *s = malloc(sizeof(StructWithPtr)); + s->memP = malloc(sizeof(int)); + free(s->memP); + free(s); +} // no-warning + +void testMallocIntoEscapedParent(StructWithPtr **out) { + StructWithPtr *s = malloc(sizeof(StructWithPtr)); + *out = s; + s->memP = malloc(sizeof(int)); +} // no-warning int conjure(void); void testExtent(void) { diff --git a/clang/test/Analysis/pr22954.c b/clang/test/Analysis/pr22954.c index b3910da6c70ab..9b5782b2fd79e 100644 --- a/clang/test/Analysis/pr22954.c +++ b/clang/test/Analysis/pr22954.c @@ -380,9 +380,11 @@ int f20(int i) { a20[1].s1[3] = 8; a20[1].s2 = strdup("world"); a20[i].s2 = strdup("hola"); - char input[] = {'a', 'b', 'c', 'd'}; + char input[] = {'a', 'b', 'c', 'd'}; // expected-warning{{Potential leak of memory pointed to by field 's2'}}\ + expected-warning{{Potential leak of memory pointed to by field 's2'}} memcpy(a20[0].s1, input, 4); - clang_analyzer_eval(a20[0].s1[0] == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(a20[0].s1[0] == 1); // expected-warning{{UNKNOWN}}\ + expected-warning{{Potential leak of memory pointed to by field 's2'}} clang_analyzer_eval(a20[0].s1[1] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(a20[0].s1[2] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(a20[0].s1[3] == 1); // expected-warning{{UNKNOWN}} @@ -422,7 +424,8 @@ int f21(int i) { a21[i].s1[3] = 4; char input[] = {'a', 'b', 'c', 'd'}; memcpy(a21[i].s1, input, 4); - clang_analyzer_eval(a21[0].s1[0] == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(a21[0].s1[0] == 1); // expected-warning{{UNKNOWN}}\ + expected-warning{{Potential leak of memory pointed to by field 's2'}} clang_analyzer_eval(a21[0].s1[1] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(a21[0].s1[2] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(a21[0].s1[3] == 1); // expected-warning{{UNKNOWN}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
