[clang] Fix for false positive issue (PR #155131)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 23 19:46:07 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Vidur (vidur2)
<details>
<summary>Changes</summary>
Hi, I am working on a PR for issue #<!-- -->153300. Currently I dont have a regression test or anything for this yet. This is just the initial fix.
---
Patch is 29.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/155131.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+160-135)
- (modified) clang/test/Analysis/NewDeleteLeaks.cpp (+140)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index efb980962e811..eda1c73b6e029 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -231,12 +231,15 @@ class RefState {
LLVM_DUMP_METHOD void dump(raw_ostream &OS) const {
switch (K) {
-#define CASE(ID) case ID: OS << #ID; break;
- CASE(Allocated)
- CASE(AllocatedOfSizeZero)
- CASE(Released)
- CASE(Relinquished)
- CASE(Escaped)
+#define CASE(ID) \
+ case ID: \
+ OS << #ID; \
+ break;
+ CASE(Allocated)
+ CASE(AllocatedOfSizeZero)
+ CASE(Released)
+ CASE(Relinquished)
+ CASE(Escaped)
}
}
@@ -304,8 +307,7 @@ struct ReallocPair {
ID.AddPointer(ReallocatedSym);
}
bool operator==(const ReallocPair &X) const {
- return ReallocatedSym == X.ReallocatedSym &&
- Kind == X.Kind;
+ return ReallocatedSym == X.ReallocatedSym && Kind == X.Kind;
}
};
@@ -422,27 +424,28 @@ class MallocChecker
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
- void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
+ void checkPostObjCMessage(const ObjCMethodCall &Call,
+ CheckerContext &C) const;
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
- bool Assumption) const;
+ bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
CheckerContext &C) const;
ProgramStateRef checkPointerEscape(ProgramStateRef State,
- const InvalidatedSymbols &Escaped,
- const CallEvent *Call,
- PointerEscapeKind Kind) const;
+ const InvalidatedSymbols &Escaped,
+ const CallEvent *Call,
+ PointerEscapeKind Kind) const;
ProgramStateRef checkConstPointerEscape(ProgramStateRef State,
const InvalidatedSymbols &Escaped,
const CallEvent *Call,
PointerEscapeKind Kind) const;
- void printState(raw_ostream &Out, ProgramStateRef State,
- const char *NL, const char *Sep) const override;
+ void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+ const char *Sep) const override;
StringRef getDebugTag() const override { return "MallocChecker"; }
@@ -789,9 +792,10 @@ class MallocChecker
///
/// We assume that pointers do not escape through calls to system functions
/// not handled by this checker.
- bool mayFreeAnyEscapedMemoryOrIsModeledExplicitly(const CallEvent *Call,
- ProgramStateRef State,
- SymbolRef &EscapingSymbol) const;
+ bool
+ mayFreeAnyEscapedMemoryOrIsModeledExplicitly(const CallEvent *Call,
+ ProgramStateRef State,
+ SymbolRef &EscapingSymbol) const;
/// Implementation of the checkPointerEscape callbacks.
[[nodiscard]] ProgramStateRef
@@ -1253,9 +1257,8 @@ MallocChecker::performKernelMalloc(const CallEvent &Call, CheckerContext &C,
NonLoc ZeroFlag = C.getSValBuilder()
.makeIntVal(*KernelZeroFlagVal, FlagsEx->getType())
.castAs<NonLoc>();
- SVal MaskedFlagsUC = C.getSValBuilder().evalBinOpNN(State, BO_And,
- Flags, ZeroFlag,
- FlagsEx->getType());
+ SVal MaskedFlagsUC = C.getSValBuilder().evalBinOpNN(
+ State, BO_And, Flags, ZeroFlag, FlagsEx->getType());
if (MaskedFlagsUC.isUnknownOrUndef())
return std::nullopt;
DefinedSVal MaskedFlags = MaskedFlagsUC.castAs<DefinedSVal>();
@@ -1485,7 +1488,8 @@ void MallocChecker::checkGMemdup(ProgramStateRef State, const CallEvent &Call,
void MallocChecker::checkGMallocN(ProgramStateRef State, const CallEvent &Call,
CheckerContext &C) const {
SVal Init = UndefinedVal();
- SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
+ SVal TotalSize =
+ evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
State = MallocMemAux(C, Call, TotalSize, Init, State,
AllocationFamily(AF_Malloc));
State = ProcessZeroAllocCheck(C, Call, 0, State);
@@ -1497,7 +1501,8 @@ void MallocChecker::checkGMallocN0(ProgramStateRef State, const CallEvent &Call,
CheckerContext &C) const {
SValBuilder &SB = C.getSValBuilder();
SVal Init = SB.makeZeroVal(SB.getContext().CharTy);
- SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
+ SVal TotalSize =
+ evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
State = MallocMemAux(C, Call, TotalSize, Init, State,
AllocationFamily(AF_Malloc));
State = ProcessZeroAllocCheck(C, Call, 0, State);
@@ -2061,8 +2066,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
/// Checks if the previous call to free on the given symbol failed - if free
/// failed, returns true. Also, returns the corresponding return value symbol.
-static bool didPreviousFreeFail(ProgramStateRef State,
- SymbolRef Sym, SymbolRef &RetStatusSymbol) {
+static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym,
+ SymbolRef &RetStatusSymbol) {
const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
if (Ret) {
assert(*Ret && "We should not store the null return symbol");
@@ -2318,8 +2323,7 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
// Check if the memory location being freed is the actual location
// allocated, or an offset.
RegionOffset Offset = R->getAsOffset();
- if (Offset.isValid() &&
- !Offset.hasSymbolicOffset() &&
+ if (Offset.isValid() && !Offset.hasSymbolicOffset() &&
Offset.getOffset() != 0) {
const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
HandleOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
@@ -2365,9 +2369,8 @@ MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
// Normal free.
if (Hold)
- return State->set<RegionState>(SymBase,
- RefState::getRelinquished(Family,
- ParentExpr));
+ return State->set<RegionState>(
+ SymBase, RefState::getRelinquished(Family, ParentExpr));
return State->set<RegionState>(SymBase,
RefState::getReleased(Family, ParentExpr));
@@ -2606,12 +2609,12 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C,
os << " allocated by " << AllocOs.str();
os << " should be deallocated by ";
- printExpectedDeallocName(os, RS->getAllocationFamily());
+ printExpectedDeallocName(os, RS->getAllocationFamily());
- if (printMemFnName(DeallocOs, C, DeallocExpr))
- os << ", not " << DeallocOs.str();
+ if (printMemFnName(DeallocOs, C, DeallocExpr))
+ os << ", not " << DeallocOs.str();
- printOwnershipTakesList(os, C, DeallocExpr);
+ printOwnershipTakesList(os, C, DeallocExpr);
}
auto R = std::make_unique<PathSensitiveBugReport>(
@@ -2648,8 +2651,7 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
assert(MR && "Only MemRegion based symbols can have offset free errors");
RegionOffset Offset = MR->getAsOffset();
- assert((Offset.isValid() &&
- !Offset.hasSymbolicOffset() &&
+ assert((Offset.isValid() && !Offset.hasSymbolicOffset() &&
Offset.getOffset() != 0) &&
"Only symbols with a valid offset can have offset free errors");
@@ -2658,11 +2660,8 @@ void MallocChecker::HandleOffsetFree(CheckerContext &C, SVal ArgVal,
os << "Argument to ";
if (!printMemFnName(os, C, DeallocExpr))
os << "deallocator";
- os << " is offset by "
- << offsetBytes
- << " "
- << ((abs(offsetBytes) > 1) ? "bytes" : "byte")
- << " from the start of ";
+ os << " is offset by " << offsetBytes << " "
+ << ((abs(offsetBytes) > 1) ? "bytes" : "byte") << " from the start of ";
if (AllocExpr && printMemFnName(AllocNameOs, C, AllocExpr))
os << "memory allocated by " << AllocNameOs.str();
else
@@ -2892,8 +2891,8 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
// Record the info about the reallocated symbol so that we could properly
// process failed reallocation.
- stateRealloc = stateRealloc->set<ReallocPairs>(ToPtr,
- ReallocPair(FromPtr, Kind));
+ stateRealloc =
+ stateRealloc->set<ReallocPairs>(ToPtr, ReallocPair(FromPtr, Kind));
// The reallocated symbol should stay alive for as long as the new symbol.
C.getSymbolManager().addSymbolDependency(ToPtr, FromPtr);
return stateRealloc;
@@ -2951,8 +2950,7 @@ MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
// Allocation node, is the last node in the current or parent context in
// which the symbol was tracked.
const LocationContext *NContext = N->getLocationContext();
- if (NContext == LeakContext ||
- NContext->isParentOf(LeakContext))
+ if (NContext == LeakContext || NContext->isParentOf(LeakContext))
AllocNode = N;
N = N->pred_empty() ? nullptr : *(N->pred_begin());
}
@@ -2988,9 +2986,8 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
const Stmt *AllocationStmt = AllocNode->getStmtForDiagnostics();
if (AllocationStmt)
- LocUsedForUniqueing = PathDiagnosticLocation::createBegin(AllocationStmt,
- C.getSourceManager(),
- AllocNode->getLocationContext());
+ LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
+ AllocationStmt, C.getSourceManager(), AllocNode->getLocationContext());
SmallString<200> buf;
llvm::raw_svector_ostream os(buf);
@@ -3012,8 +3009,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
}
void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
- CheckerContext &C) const
-{
+ CheckerContext &C) const {
ProgramStateRef state = C.getState();
RegionStateTy OldRS = state->get<RegionState>();
RegionStateTy::Factory &F = state->get_context<RegionState>();
@@ -3031,8 +3027,7 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
if (RS == OldRS) {
// We shouldn't have touched other maps yet.
- assert(state->get<ReallocPairs>() ==
- C.getState()->get<ReallocPairs>());
+ assert(state->get<ReallocPairs>() == C.getState()->get<ReallocPairs>());
assert(state->get<FreeReturnValue>() ==
C.getState()->get<FreeReturnValue>());
return;
@@ -3074,6 +3069,43 @@ void MallocChecker::checkPostCall(const CallEvent &Call,
(*PostFN)(this, C.getState(), Call, C);
return;
}
+
+ ProgramStateRef State = C.getState();
+
+ if (const auto *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
+ // Ensure we are constructing a concrete object/subobject.
+ if (const MemRegion *ObjUnderConstr = Ctor->getCXXThisVal().getAsRegion()) {
+ ProgramStateRef NewState = State;
+
+ for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) {
+ SVal ArgV = Call.getArgSVal(I);
+
+ SymbolRef Sym = ArgV.getAsSymbol();
+ if (!Sym)
+ continue;
+
+ // Look up current ref-state for this symbol in the RegionState map.
+ if (const RefState *RS = State->get<RegionState>(Sym)) {
+ // Only re-label symbols that are still owned allocations from C++
+ // new/new[].
+ if (RS->isAllocated() &&
+ (RS->getAllocationFamily().Kind == AF_CXXNew ||
+ RS->getAllocationFamily().Kind == AF_CXXNewArray)) {
+
+ // Mark as Relinquished at the constructor site: ownership moves
+ // into the constructed subobject. Pass the ctor's origin expr as
+ // the statement associated with this transition.
+ NewState = NewState->set<RegionState>(
+ Sym, RefState::getRelinquished(RS->getAllocationFamily(),
+ Ctor->getOriginExpr()));
+ }
+ }
+ }
+
+ if (NewState != State)
+ C.addTransition(NewState);
+ }
+ }
}
void MallocChecker::checkPreCall(const CallEvent &Call,
@@ -3165,8 +3197,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
}
}
-void MallocChecker::checkPreStmt(const ReturnStmt *S,
- CheckerContext &C) const {
+void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
checkEscapeOnReturn(S, C);
}
@@ -3198,7 +3229,7 @@ void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
if (const MemRegion *MR = RetVal.getAsRegion())
if (isa<FieldRegion, ElementRegion>(MR))
if (const SymbolicRegion *BMR =
- dyn_cast<SymbolicRegion>(MR->getBaseRegion()))
+ dyn_cast<SymbolicRegion>(MR->getBaseRegion()))
Sym = BMR->getSymbol();
// Check if we are returning freed memory.
@@ -3218,14 +3249,13 @@ void MallocChecker::checkPostStmt(const BlockExpr *BE,
return;
ProgramStateRef state = C.getState();
- const BlockDataRegion *R =
- cast<BlockDataRegion>(C.getSVal(BE).getAsRegion());
+ const BlockDataRegion *R = cast<BlockDataRegion>(C.getSVal(BE).getAsRegion());
auto ReferencedVars = R->referenced_vars();
if (ReferencedVars.empty())
return;
- SmallVector<const MemRegion*, 10> Regions;
+ SmallVector<const MemRegion *, 10> Regions;
const LocationContext *LC = C.getLocationContext();
MemRegionManager &MemMgr = C.getSValBuilder().getRegionManager();
@@ -3237,8 +3267,7 @@ void MallocChecker::checkPostStmt(const BlockExpr *BE,
Regions.push_back(VR);
}
- state =
- state->scanReachableSymbols<StopTrackingCallback>(Regions).getState();
+ state = state->scanReachableSymbols<StopTrackingCallback>(Regions).getState();
C.addTransition(state);
}
@@ -3295,8 +3324,7 @@ void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
if (const RefState *RS = C.getState()->get<RegionState>(Sym)) {
if (RS->isAllocatedOfSizeZero())
HandleUseZeroAlloc(C, RS->getStmt()->getSourceRange(), Sym);
- }
- else if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym)) {
+ } else if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym)) {
HandleUseZeroAlloc(C, S->getSourceRange(), Sym);
}
}
@@ -3313,9 +3341,8 @@ void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S,
// If a symbolic region is assumed to NULL (or another constant), stop tracking
// it - assuming that allocation failed on this path.
-ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
- SVal Cond,
- bool Assumption) const {
+ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, SVal Cond,
+ bool Assumption) const {
RegionStateTy RS = state->get<RegionState>();
for (SymbolRef Sym : llvm::make_first_range(RS)) {
// If the symbol is assumed to be NULL, remove it from consideration.
@@ -3340,7 +3367,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
if (RS->isReleased()) {
switch (ReallocPair.Kind) {
case OAR_ToBeFreedAfterFailure:
- state = state->set<RegionState>(ReallocSym,
+ state = state->set<RegionState>(
+ ReallocSym,
RefState::getAllocated(RS->getAllocationFamily(), RS->getStmt()));
break;
case OAR_DoNotTrackAfterFailure:
@@ -3358,9 +3386,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
}
bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
- const CallEvent *Call,
- ProgramStateRef State,
- SymbolRef &EscapingSymbol) const {
+ const CallEvent *Call, ProgramStateRef State,
+ SymbolRef &EscapingSymbol) const {
assert(Call);
EscapingSymbol = nullptr;
@@ -3470,8 +3497,8 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
// Do not warn on pointers passed to 'setbuf' when used with std streams,
// these leaks might be intentional when setting the buffer for stdio.
// http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer
- if (FName == "setbuf" || FName =="setbuffer" ||
- FName == "setlinebuf" || FName == "setvbuf") {
+ if (FName == "setbuf" || FName == "setbuffer" || FName == "setlinebuf" ||
+ FName == "setvbuf") {
if (Call->getNumArgs() >= 1) {
const Expr *ArgE = Call->getArgExpr(0)->IgnoreParenCasts();
if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE))
@@ -3521,18 +3548,16 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
return false;
}
-ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
- const InvalidatedSymbols &Escaped,
- const CallEvent *Call,
- PointerEscapeKind Kind) const {
+ProgramStateRef MallocChecker::checkPointerEscape(
+ ProgramStateRef State, const InvalidatedSymbols &Escaped,
+ const CallEvent *Call, PointerEscapeKind Kind) const {
return checkPointerEscapeAux(State, Escaped, Call, Kind,
/*IsConstPointerEscape*/ false);
}
-ProgramStateRef MallocChecker::checkConstPointerEscape(ProgramStateRef State,
- const InvalidatedSymbols &Escaped,
- const CallEvent *Call,
- PointerEscapeKind Kind) const {
+ProgramStateRef MallocChecker::checkConstPointerEscape(
+ ProgramStateRef State, const InvalidatedSymbols &Escaped,
+ const CallEvent *Call, PointerEscapeKind Kind) const {
// If a const pointer escapes, it may not be freed(), but it could be deleted.
return checkPointerEscapeAux(State, Escaped, Call, Kind,
/*IsConstPointerEscape*/ true);
@@ -3724,61 +3749,61 @@ PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N,
}
Msg = OS.str();
break;
- }
- case AF_None:
- assert(false && "Unhandled allocation family!");
- return nullptr;
- }
+ }
+ case AF_None:
+ assert(false && "Unhandled allocation family!");
+ return nullptr;
+ }
- // 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
...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/155131
More information about the cfe-commits
mailing list