Skip to content

Commit 629ce99

Browse files
ivanmurashkosteakhal
authored andcommitted
[analyzer] MallocChecker – Fix false positive leak for smart pointers in temporary objects (llvm#152751)
Fixes PR60896 - false positive leak reports in various smart pointer scenarios including temporaries, inheritance, direct constructor calls, and mixed ownership patterns. Previously, the analyzer had no smart pointer handling in `checkPostCall`, causing it to report false positive leaks for memory properly managed by smart pointers while missing legitimate raw pointer leaks. --------- Co-authored-by: Balazs Benics <[email protected]> Co-authored-by: Donát Nagy <[email protected]> (cherry picked from commit ff9b296) Fixes rdar://163052948
1 parent 68b6ffe commit 629ce99

File tree

2 files changed

+557
-2
lines changed

2 files changed

+557
-2
lines changed

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 305 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
7979
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
8080
#include "llvm/ADT/STLExtras.h"
81+
#include "llvm/ADT/SmallVector.h"
8182
#include "llvm/ADT/StringExtras.h"
8283
#include "llvm/Support/Casting.h"
8384
#include "llvm/Support/Compiler.h"
@@ -421,6 +422,13 @@ class MallocChecker
421422
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
422423
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
423424
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
425+
426+
ProgramStateRef
427+
handleSmartPointerConstructorArguments(const CallEvent &Call,
428+
ProgramStateRef State) const;
429+
ProgramStateRef handleSmartPointerRelatedCalls(const CallEvent &Call,
430+
CheckerContext &C,
431+
ProgramStateRef State) const;
424432
void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
425433
void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
426434
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
@@ -1096,6 +1104,54 @@ class StopTrackingCallback final : public SymbolVisitor {
10961104
return true;
10971105
}
10981106
};
1107+
1108+
/// EscapeTrackedCallback - A SymbolVisitor that marks allocated symbols as
1109+
/// escaped.
1110+
///
1111+
/// This visitor is used to suppress false positive leak reports when smart
1112+
/// pointers are nested in temporary objects passed by value to functions. When
1113+
/// the analyzer can't see the destructor calls for temporary objects, it may
1114+
/// incorrectly report leaks for memory that will be properly freed by the smart
1115+
/// pointer destructors.
1116+
///
1117+
/// The visitor traverses reachable symbols from a given set of memory regions
1118+
/// (typically smart pointer field regions) and marks any allocated symbols as
1119+
/// escaped. Escaped symbols are not reported as leaks by checkDeadSymbols.
1120+
class EscapeTrackedCallback final : public SymbolVisitor {
1121+
ProgramStateRef State;
1122+
1123+
explicit EscapeTrackedCallback(ProgramStateRef S) : State(std::move(S)) {}
1124+
1125+
public:
1126+
bool VisitSymbol(SymbolRef Sym) override {
1127+
if (const RefState *RS = State->get<RegionState>(Sym)) {
1128+
if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) {
1129+
State = State->set<RegionState>(Sym, RefState::getEscaped(RS));
1130+
}
1131+
}
1132+
return true;
1133+
}
1134+
1135+
/// Escape tracked regions reachable from the given roots.
1136+
static ProgramStateRef
1137+
EscapeTrackedRegionsReachableFrom(ArrayRef<const MemRegion *> Roots,
1138+
ProgramStateRef State) {
1139+
if (Roots.empty())
1140+
return State;
1141+
1142+
// scanReachableSymbols is expensive, so we use a single visitor for all
1143+
// roots
1144+
SmallVector<const MemRegion *, 10> Regions;
1145+
EscapeTrackedCallback Visitor(State);
1146+
for (const MemRegion *R : Roots) {
1147+
Regions.push_back(R);
1148+
}
1149+
State->scanReachableSymbols(Regions, Visitor);
1150+
return Visitor.State;
1151+
}
1152+
1153+
friend class SymbolVisitor;
1154+
};
10991155
} // end anonymous namespace
11001156

11011157
static bool isStandardNew(const FunctionDecl *FD) {
@@ -3071,12 +3127,260 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
30713127
C.addTransition(state->set<RegionState>(RS), N);
30723128
}
30733129

3130+
// Allowlist of owning smart pointers we want to recognize.
3131+
// Start with unique_ptr and shared_ptr; weak_ptr is excluded intentionally
3132+
// because it does not own the pointee.
3133+
static bool isSmartPtrName(StringRef Name) {
3134+
return Name == "unique_ptr" || Name == "shared_ptr";
3135+
}
3136+
3137+
// Check if a type is a smart owning pointer type.
3138+
static bool isSmartPtrType(QualType QT) {
3139+
QT = QT->getCanonicalTypeUnqualified();
3140+
3141+
if (const auto *TST = QT->getAs<TemplateSpecializationType>()) {
3142+
const TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl();
3143+
if (!TD)
3144+
return false;
3145+
3146+
const auto *ND = dyn_cast_or_null<NamedDecl>(TD->getTemplatedDecl());
3147+
if (!ND)
3148+
return false;
3149+
3150+
// For broader coverage we recognize all template classes with names that
3151+
// match the allowlist even if they are not declared in namespace 'std'.
3152+
return isSmartPtrName(ND->getName());
3153+
}
3154+
3155+
return false;
3156+
}
3157+
3158+
/// Helper struct for collecting smart owning pointer field regions.
3159+
/// This allows both hasSmartPtrField and
3160+
/// collectSmartPtrFieldRegions to share the same traversal logic,
3161+
/// ensuring consistency.
3162+
struct FieldConsumer {
3163+
const MemRegion *Reg;
3164+
CheckerContext *C;
3165+
llvm::SmallPtrSetImpl<const MemRegion *> *Out;
3166+
3167+
FieldConsumer(const MemRegion *Reg, CheckerContext &C,
3168+
llvm::SmallPtrSetImpl<const MemRegion *> &Out)
3169+
: Reg(Reg), C(&C), Out(&Out) {}
3170+
3171+
void consume(const FieldDecl *FD) {
3172+
SVal L = C->getState()->getLValue(FD, loc::MemRegionVal(Reg));
3173+
if (const MemRegion *FR = L.getAsRegion())
3174+
Out->insert(FR);
3175+
}
3176+
3177+
std::optional<FieldConsumer> switchToBase(const CXXRecordDecl *BaseDecl,
3178+
bool IsVirtual) {
3179+
// Get the base class region
3180+
SVal BaseL =
3181+
C->getState()->getLValue(BaseDecl, Reg->getAs<SubRegion>(), IsVirtual);
3182+
if (const MemRegion *BaseObjRegion = BaseL.getAsRegion()) {
3183+
// Return a consumer for the base class
3184+
return FieldConsumer{BaseObjRegion, *C, *Out};
3185+
}
3186+
return std::nullopt;
3187+
}
3188+
};
3189+
3190+
/// Check if a record type has smart owning pointer fields (directly or in base
3191+
/// classes). When FC is provided, also collect the field regions.
3192+
///
3193+
/// This function has dual behavior:
3194+
/// - When FC is nullopt: Returns true if smart pointer fields are found
3195+
/// - When FC is provided: Always returns false, but collects field regions
3196+
/// as a side effect through the FieldConsumer
3197+
///
3198+
/// Note: When FC is provided, the return value should be ignored since the
3199+
/// function performs full traversal for collection and always returns false
3200+
/// to avoid early termination.
3201+
static bool hasSmartPtrField(const CXXRecordDecl *CRD,
3202+
std::optional<FieldConsumer> FC = std::nullopt) {
3203+
// Check direct fields
3204+
for (const FieldDecl *FD : CRD->fields()) {
3205+
if (isSmartPtrType(FD->getType())) {
3206+
if (!FC)
3207+
return true;
3208+
FC->consume(FD);
3209+
}
3210+
}
3211+
3212+
// Check fields from base classes
3213+
for (const CXXBaseSpecifier &BaseSpec : CRD->bases()) {
3214+
if (const CXXRecordDecl *BaseDecl =
3215+
BaseSpec.getType()->getAsCXXRecordDecl()) {
3216+
std::optional<FieldConsumer> NewFC;
3217+
if (FC) {
3218+
NewFC = FC->switchToBase(BaseDecl, BaseSpec.isVirtual());
3219+
if (!NewFC)
3220+
continue;
3221+
}
3222+
bool Found = hasSmartPtrField(BaseDecl, NewFC);
3223+
if (Found && !FC)
3224+
return true;
3225+
}
3226+
}
3227+
return false;
3228+
}
3229+
3230+
/// Check if an expression is an rvalue record type passed by value.
3231+
static bool isRvalueByValueRecord(const Expr *AE) {
3232+
if (AE->isGLValue())
3233+
return false;
3234+
3235+
QualType T = AE->getType();
3236+
if (!T->isRecordType() || T->isReferenceType())
3237+
return false;
3238+
3239+
// Accept common temp/construct forms but don't overfit.
3240+
return isa<CXXTemporaryObjectExpr, MaterializeTemporaryExpr, CXXConstructExpr,
3241+
InitListExpr, ImplicitCastExpr, CXXBindTemporaryExpr>(AE);
3242+
}
3243+
3244+
/// Check if an expression is an rvalue record with smart owning pointer fields
3245+
/// passed by value.
3246+
static bool isRvalueByValueRecordWithSmartPtr(const Expr *AE) {
3247+
if (!isRvalueByValueRecord(AE))
3248+
return false;
3249+
3250+
const auto *CRD = AE->getType()->getAsCXXRecordDecl();
3251+
return CRD && hasSmartPtrField(CRD);
3252+
}
3253+
3254+
/// Check if a CXXRecordDecl has a name matching recognized smart pointer names.
3255+
static bool isSmartPtrRecord(const CXXRecordDecl *RD) {
3256+
if (!RD)
3257+
return false;
3258+
3259+
// Check the record name directly and accept both std and custom smart pointer
3260+
// implementations for broader coverage
3261+
return isSmartPtrName(RD->getName());
3262+
}
3263+
3264+
/// Check if a call is a constructor of a smart owning pointer class that
3265+
/// accepts pointer parameters.
3266+
static bool isSmartPtrCall(const CallEvent &Call) {
3267+
// Only check for smart pointer constructor calls
3268+
const auto *CD = dyn_cast_or_null<CXXConstructorDecl>(Call.getDecl());
3269+
if (!CD)
3270+
return false;
3271+
3272+
const auto *RD = CD->getParent();
3273+
if (!isSmartPtrRecord(RD))
3274+
return false;
3275+
3276+
// Check if constructor takes a pointer parameter
3277+
for (const auto *Param : CD->parameters()) {
3278+
QualType ParamType = Param->getType();
3279+
if (ParamType->isPointerType() && !ParamType->isFunctionPointerType() &&
3280+
!ParamType->isVoidPointerType()) {
3281+
return true;
3282+
}
3283+
}
3284+
3285+
return false;
3286+
}
3287+
3288+
/// Collect memory regions of smart owning pointer fields from a record type
3289+
/// (including fields from base classes).
3290+
static void
3291+
collectSmartPtrFieldRegions(const MemRegion *Reg, QualType RecQT,
3292+
CheckerContext &C,
3293+
llvm::SmallPtrSetImpl<const MemRegion *> &Out) {
3294+
if (!Reg)
3295+
return;
3296+
3297+
const auto *CRD = RecQT->getAsCXXRecordDecl();
3298+
if (!CRD)
3299+
return;
3300+
3301+
FieldConsumer FC{Reg, C, Out};
3302+
hasSmartPtrField(CRD, FC);
3303+
}
3304+
3305+
/// Handle smart pointer constructor calls by escaping allocated symbols
3306+
/// that are passed as pointer arguments to the constructor.
3307+
ProgramStateRef MallocChecker::handleSmartPointerConstructorArguments(
3308+
const CallEvent &Call, ProgramStateRef State) const {
3309+
const auto *CD = cast<CXXConstructorDecl>(Call.getDecl());
3310+
for (unsigned I = 0, E = std::min(Call.getNumArgs(), CD->getNumParams());
3311+
I != E; ++I) {
3312+
const Expr *ArgExpr = Call.getArgExpr(I);
3313+
if (!ArgExpr)
3314+
continue;
3315+
3316+
QualType ParamType = CD->getParamDecl(I)->getType();
3317+
if (ParamType->isPointerType() && !ParamType->isFunctionPointerType() &&
3318+
!ParamType->isVoidPointerType()) {
3319+
// This argument is a pointer being passed to smart pointer constructor
3320+
SVal ArgVal = Call.getArgSVal(I);
3321+
SymbolRef Sym = ArgVal.getAsSymbol();
3322+
if (Sym && State->contains<RegionState>(Sym)) {
3323+
const RefState *RS = State->get<RegionState>(Sym);
3324+
if (RS && (RS->isAllocated() || RS->isAllocatedOfSizeZero())) {
3325+
State = State->set<RegionState>(Sym, RefState::getEscaped(RS));
3326+
}
3327+
}
3328+
}
3329+
}
3330+
return State;
3331+
}
3332+
3333+
/// Handle all smart pointer related processing in function calls.
3334+
/// This includes both direct smart pointer constructor calls and by-value
3335+
/// arguments containing smart pointer fields.
3336+
ProgramStateRef MallocChecker::handleSmartPointerRelatedCalls(
3337+
const CallEvent &Call, CheckerContext &C, ProgramStateRef State) const {
3338+
3339+
// Handle direct smart pointer constructor calls first
3340+
if (isSmartPtrCall(Call)) {
3341+
return handleSmartPointerConstructorArguments(Call, State);
3342+
}
3343+
3344+
// Handle smart pointer fields in by-value record arguments
3345+
llvm::SmallPtrSet<const MemRegion *, 8> SmartPtrFieldRoots;
3346+
for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) {
3347+
const Expr *AE = Call.getArgExpr(I);
3348+
if (!AE)
3349+
continue;
3350+
AE = AE->IgnoreParenImpCasts();
3351+
3352+
if (!isRvalueByValueRecordWithSmartPtr(AE))
3353+
continue;
3354+
3355+
// Find a region for the argument.
3356+
SVal ArgVal = Call.getArgSVal(I);
3357+
const MemRegion *ArgRegion = ArgVal.getAsRegion();
3358+
// Collect direct smart owning pointer field regions
3359+
collectSmartPtrFieldRegions(ArgRegion, AE->getType(), C,
3360+
SmartPtrFieldRoots);
3361+
}
3362+
3363+
// Escape symbols reachable from smart pointer fields
3364+
if (!SmartPtrFieldRoots.empty()) {
3365+
SmallVector<const MemRegion *, 8> SmartPtrFieldRootsVec(
3366+
SmartPtrFieldRoots.begin(), SmartPtrFieldRoots.end());
3367+
State = EscapeTrackedCallback::EscapeTrackedRegionsReachableFrom(
3368+
SmartPtrFieldRootsVec, State);
3369+
}
3370+
3371+
return State;
3372+
}
3373+
30743374
void MallocChecker::checkPostCall(const CallEvent &Call,
30753375
CheckerContext &C) const {
3376+
// Handle existing post-call handlers first
30763377
if (const auto *PostFN = PostFnMap.lookup(Call)) {
30773378
(*PostFN)(this, C.getState(), Call, C);
3078-
return;
3379+
return; // Post-handler already called addTransition, we're done
30793380
}
3381+
3382+
// Handle smart pointer related processing only if no post-handler was called
3383+
C.addTransition(handleSmartPointerRelatedCalls(Call, C, C.getState()));
30803384
}
30813385

30823386
void MallocChecker::checkPreCall(const CallEvent &Call,
@@ -3197,7 +3501,6 @@ void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
31973501
if (!Sym)
31983502
// If we are returning a field of the allocated struct or an array element,
31993503
// the callee could still free the memory.
3200-
// TODO: This logic should be a part of generic symbol escape callback.
32013504
if (const MemRegion *MR = RetVal.getAsRegion())
32023505
if (isa<FieldRegion, ElementRegion>(MR))
32033506
if (const SymbolicRegion *BMR =

0 commit comments

Comments
 (0)