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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
