Skip to content

Commit 650b0f7

Browse files
committed
[clang-analyzer] MallocChecker: extend false positive leak fix to support shared_ptr
Extend the existing post-call escape rule to recognize std::shared_ptr<T> fields in addition to std::unique_ptr<T>. The fix suppresses false positive leaks when smart pointers are nested in temporary objects passed by value to functions. Key changes: - Replace isUniquePtrType() with isSmartOwningPtrType() that recognizes both unique_ptr and shared_ptr (both std:: and custom implementations) - Update field collection logic to use the generalized smart pointer detection - Add test coverage for shared_ptr scenarios The fix remains narrow and safe: - Only affects rvalue by-value record arguments at call sites - Only scans from direct smart pointer field regions (no mass escapes) - Inline-agnostic; no checkPreCall mutations - Intentionally excludes std::weak_ptr (non-owning) Fixes PR60896 and extends the solution to cover shared_ptr use cases.
1 parent f6d1a05 commit 650b0f7

File tree

2 files changed

+92
-44
lines changed

2 files changed

+92
-44
lines changed

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,21 @@ class StopTrackingCallback final : public SymbolVisitor {
11021102
}
11031103
};
11041104

1105+
/// EscapeTrackedCallback - A SymbolVisitor that marks allocated symbols as escaped.
1106+
///
1107+
/// This visitor is used to suppress false positive leak reports when smart pointers
1108+
/// are nested in temporary objects passed by value to functions. When the analyzer
1109+
/// can't see the destructor calls for temporary objects, it may incorrectly report
1110+
/// leaks for memory that will be properly freed by the smart pointer destructors.
1111+
///
1112+
/// The visitor traverses reachable symbols from a given set of memory regions
1113+
/// (typically smart pointer field regions) and marks any allocated symbols as
1114+
/// escaped. Escaped symbols are not reported as leaks by checkDeadSymbols.
1115+
///
1116+
/// Usage:
1117+
/// auto Scan = State->scanReachableSymbols<EscapeTrackedCallback>(RootRegions);
1118+
/// ProgramStateRef NewState = Scan.getState();
1119+
/// if (NewState != State) C.addTransition(NewState);
11051120
class EscapeTrackedCallback final : public SymbolVisitor {
11061121
ProgramStateRef State;
11071122

@@ -3104,10 +3119,12 @@ static bool isInStdNamespace(const DeclContext *DC) {
31043119
return false;
31053120
}
31063121

3107-
static bool isUniquePtrType(QualType QT) {
3122+
// Allowlist of owning smart pointers we want to recognize.
3123+
// Start with unique_ptr and shared_ptr. (intentionally exclude weak_ptr)
3124+
static bool isSmartOwningPtrType(QualType QT) {
31083125
QT = canonicalStrip(QT);
31093126

3110-
// First try TemplateSpecializationType (for std::unique_ptr)
3127+
// First try TemplateSpecializationType (for std smart pointers)
31113128
const auto *TST = QT->getAs<TemplateSpecializationType>();
31123129
if (TST) {
31133130
const TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl();
@@ -3116,38 +3133,42 @@ static bool isUniquePtrType(QualType QT) {
31163133
const auto *ND = dyn_cast_or_null<NamedDecl>(TD->getTemplatedDecl());
31173134
if (!ND) return false;
31183135

3119-
if (ND->getName() != "unique_ptr") return false;
3120-
31213136
// Check if it's in std namespace
31223137
const DeclContext *DC = ND->getDeclContext();
3123-
if (isInStdNamespace(DC)) return true;
3138+
if (!isInStdNamespace(DC)) return false;
3139+
3140+
StringRef Name = ND->getName();
3141+
return Name == "unique_ptr" || Name == "shared_ptr";
31243142
}
31253143

3126-
// Also try RecordType (for custom unique_ptr)
3144+
// Also try RecordType (for custom smart pointer implementations)
31273145
const auto *RT = QT->getAs<RecordType>();
31283146
if (RT) {
31293147
const auto *RD = RT->getDecl();
3130-
if (RD && RD->getName() == "unique_ptr") {
3131-
// Accept any custom unique_ptr implementation
3132-
return true;
3148+
if (RD) {
3149+
StringRef Name = RD->getName();
3150+
if (Name == "unique_ptr" || Name == "shared_ptr") {
3151+
// Accept any custom unique_ptr or shared_ptr implementation
3152+
return true;
3153+
}
31333154
}
31343155
}
31353156

31363157
return false;
31373158
}
31383159

3139-
static void collectDirectUniquePtrFieldRegions(const MemRegion *Base,
3140-
QualType RecQT,
3141-
ProgramStateRef State,
3142-
SmallVectorImpl<const MemRegion*> &Out) {
3160+
static void collectDirectSmartOwningPtrFieldRegions(const MemRegion *Base,
3161+
QualType RecQT,
3162+
CheckerContext &C,
3163+
SmallVectorImpl<const MemRegion*> &Out) {
31433164
if (!Base) return;
31443165
const auto *CRD = RecQT->getAsCXXRecordDecl();
31453166
if (!CRD) return;
31463167

31473168
for (const FieldDecl *FD : CRD->fields()) {
3148-
if (!isUniquePtrType(FD->getType()))
3169+
if (!isSmartOwningPtrType(FD->getType()))
31493170
continue;
3150-
SVal L = State->getLValue(FD, loc::MemRegionVal(Base));
3171+
SVal L = C.getState()->getLValue(FD, loc::MemRegionVal(Base));
31513172
if (const MemRegion *FR = L.getAsRegion())
31523173
Out.push_back(FR);
31533174
}
@@ -3160,7 +3181,7 @@ void MallocChecker::checkPostCall(const CallEvent &Call,
31603181
(*PostFN)(this, C.getState(), Call, C);
31613182
}
31623183

3163-
SmallVector<const MemRegion*, 8> UniquePtrFieldRoots;
3184+
SmallVector<const MemRegion*, 8> SmartPtrFieldRoots;
31643185

31653186

31663187

@@ -3187,17 +3208,17 @@ void MallocChecker::checkPostCall(const CallEvent &Call,
31873208
isa<CXXBindTemporaryExpr>(AE); // handle CXXBindTemporaryExpr
31883209
if (!LooksLikeTemp) continue;
31893210

3190-
// Require at least one direct unique_ptr field by type.
3211+
// Require at least one direct smart owning pointer field by type.
31913212
const auto *CRD = T->getAsCXXRecordDecl();
31923213
if (!CRD) continue;
3193-
bool HasUPtrField = false;
3214+
bool HasSmartPtrField = false;
31943215
for (const FieldDecl *FD : CRD->fields()) {
3195-
if (isUniquePtrType(FD->getType())) {
3196-
HasUPtrField = true;
3216+
if (isSmartOwningPtrType(FD->getType())) {
3217+
HasSmartPtrField = true;
31973218
break;
31983219
}
31993220
}
3200-
if (!HasUPtrField) continue;
3221+
if (!HasSmartPtrField) continue;
32013222

32023223
// Find a region for the argument.
32033224
SVal VCall = Call.getArgSVal(I);
@@ -3222,21 +3243,21 @@ void MallocChecker::checkPostCall(const CallEvent &Call,
32223243
continue;
32233244
}
32243245

3225-
// Push direct unique_ptr field regions only (precise root set).
3226-
collectDirectUniquePtrFieldRegions(Base, T, C.getState(), UniquePtrFieldRoots);
3246+
// Push direct smart owning pointer field regions only (precise root set).
3247+
collectDirectSmartOwningPtrFieldRegions(Base, T, C, SmartPtrFieldRoots);
32273248
}
32283249

32293250
// Escape only from those field roots; do nothing if empty.
3230-
if (!UniquePtrFieldRoots.empty()) {
3251+
if (!SmartPtrFieldRoots.empty()) {
32313252
ProgramStateRef State = C.getState();
3232-
auto Scan = State->scanReachableSymbols<EscapeTrackedCallback>(UniquePtrFieldRoots);
3253+
auto Scan = State->scanReachableSymbols<EscapeTrackedCallback>(SmartPtrFieldRoots);
32333254
ProgramStateRef NewState = Scan.getState();
32343255
if (NewState != State) {
32353256
C.addTransition(NewState);
32363257
} else {
3237-
// Fallback: if we have by-value record arguments but no unique_ptr fields detected,
3238-
// check if any of the arguments are by-value records with unique_ptr fields
3239-
bool hasByValueRecordWithUniquePtr = false;
3258+
// Fallback: if we have by-value record arguments but no smart pointer fields detected,
3259+
// check if any of the arguments are by-value records with smart pointer fields
3260+
bool hasByValueRecordWithSmartPtr = false;
32403261
for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) {
32413262
const Expr *AE = Call.getArgExpr(I);
32423263
if (!AE) continue;
@@ -3255,20 +3276,20 @@ void MallocChecker::checkPostCall(const CallEvent &Call,
32553276
isa<CXXBindTemporaryExpr>(AE);
32563277
if (!LooksLikeTemp) continue;
32573278

3258-
// Check if this record type has unique_ptr fields
3279+
// Check if this record type has smart pointer fields
32593280
const auto *CRD = T->getAsCXXRecordDecl();
32603281
if (CRD) {
32613282
for (const FieldDecl *FD : CRD->fields()) {
3262-
if (isUniquePtrType(FD->getType())) {
3263-
hasByValueRecordWithUniquePtr = true;
3283+
if (isSmartOwningPtrType(FD->getType())) {
3284+
hasByValueRecordWithSmartPtr = true;
32643285
break;
32653286
}
32663287
}
32673288
}
3268-
if (hasByValueRecordWithUniquePtr) break;
3289+
if (hasByValueRecordWithSmartPtr) break;
32693290
}
32703291

3271-
if (hasByValueRecordWithUniquePtr) {
3292+
if (hasByValueRecordWithSmartPtr) {
32723293
ProgramStateRef State = C.getState();
32733294
RegionStateTy RS = State->get<RegionState>();
32743295
ProgramStateRef NewState = State;
@@ -3346,17 +3367,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
33463367
if (!FD)
33473368
return;
33483369

3349-
// If we won't inline this call, conservatively treat by-value record
3350-
// arguments as escaping any tracked pointers they contain.
3351-
const bool WillNotInline = !FD || !FD->hasBody();
3352-
if (WillNotInline) {
3353-
// TODO: Implement proper escape logic for by-value record arguments
3354-
// The issue is that when a record type is passed by value to a non-inlined
3355-
// function, the analyzer doesn't see the destructor calls for the temporary
3356-
// object, leading to false positive leaks. We need to mark contained
3357-
// pointers as escaped in such cases.
3358-
// For now, just skip this to avoid crashes
3359-
}
3370+
33603371

33613372
// FIXME: I suspect we should remove `MallocChecker.isEnabled() &&` because
33623373
// it's fishy that the enabled/disabled state of one frontend may influence
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,unix -verify %s
2+
// expected-no-diagnostics
3+
4+
#include "Inputs/system-header-simulator-for-malloc.h"
5+
6+
// Test shared_ptr support in the same pattern as the original PR60896 test
7+
namespace shared_ptr_test {
8+
9+
template <typename T>
10+
struct shared_ptr {
11+
T* ptr;
12+
shared_ptr(T* p) : ptr(p) {}
13+
~shared_ptr() { delete ptr; }
14+
shared_ptr(shared_ptr&& other) : ptr(other.ptr) { other.ptr = nullptr; }
15+
T* get() const { return ptr; }
16+
};
17+
18+
template <typename T, typename... Args>
19+
shared_ptr<T> make_shared(Args&&... args) {
20+
return shared_ptr<T>(new T(args...));
21+
}
22+
23+
struct Foo {
24+
shared_ptr<int> i;
25+
};
26+
27+
void add(Foo foo) {
28+
// The shared_ptr destructor will be called when foo goes out of scope
29+
}
30+
31+
void test() {
32+
// No warning should be emitted for this - the memory is managed by shared_ptr
33+
// in the temporary Foo object, which will properly clean up the memory
34+
add({make_shared<int>(1)});
35+
}
36+
37+
} // namespace shared_ptr_test

0 commit comments

Comments
 (0)