-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[analyzer] MallocChecker – Fix false positive leak for smart pointers in temporary objects #152751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7d1c700
f6d1a05
650b0f7
0cc838f
333e5ba
07cfed9
66bf4e6
2dc6776
83173d0
fddc1b4
617a02a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,7 @@ | |
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" | ||
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" | ||
#include "llvm/ADT/STLExtras.h" | ||
#include "llvm/ADT/SmallVector.h" | ||
#include "llvm/ADT/StringExtras.h" | ||
#include "llvm/Support/Casting.h" | ||
#include "llvm/Support/Compiler.h" | ||
|
@@ -1096,6 +1097,54 @@ class StopTrackingCallback final : public SymbolVisitor { | |
return true; | ||
} | ||
}; | ||
|
||
/// EscapeTrackedCallback - A SymbolVisitor that marks allocated symbols as | ||
/// escaped. | ||
/// | ||
/// This visitor is used to suppress false positive leak reports when smart | ||
/// pointers are nested in temporary objects passed by value to functions. When | ||
/// the analyzer can't see the destructor calls for temporary objects, it may | ||
/// incorrectly report leaks for memory that will be properly freed by the smart | ||
/// pointer destructors. | ||
/// | ||
/// The visitor traverses reachable symbols from a given set of memory regions | ||
/// (typically smart pointer field regions) and marks any allocated symbols as | ||
/// escaped. Escaped symbols are not reported as leaks by checkDeadSymbols. | ||
class EscapeTrackedCallback final : public SymbolVisitor { | ||
ProgramStateRef State; | ||
|
||
explicit EscapeTrackedCallback(ProgramStateRef S) : State(std::move(S)) {} | ||
|
||
bool VisitSymbol(SymbolRef Sym) override { | ||
if (const RefState *RS = State->get<RegionState>(Sym)) { | ||
if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) { | ||
State = State->set<RegionState>(Sym, RefState::getEscaped(RS)); | ||
} | ||
} | ||
return true; | ||
} | ||
|
||
public: | ||
/// Escape tracked regions reachable from the given roots. | ||
static ProgramStateRef | ||
EscapeTrackedRegionsReachableFrom(ArrayRef<const MemRegion *> Roots, | ||
ProgramStateRef State) { | ||
if (Roots.empty()) | ||
return State; | ||
|
||
// scanReachableSymbols is expensive, so we use a single visitor for all | ||
// roots | ||
SmallVector<const MemRegion *, 10> Regions; | ||
EscapeTrackedCallback Visitor(State); | ||
for (const MemRegion *R : Roots) { | ||
Regions.push_back(R); | ||
} | ||
State->scanReachableSymbols(Regions, Visitor); | ||
return Visitor.State; | ||
} | ||
|
||
friend class SymbolVisitor; | ||
}; | ||
} // end anonymous namespace | ||
|
||
static bool isStandardNew(const FunctionDecl *FD) { | ||
|
@@ -3068,12 +3117,167 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, | |
C.addTransition(state->set<RegionState>(RS), N); | ||
} | ||
|
||
// Allowlist of owning smart pointers we want to recognize. | ||
// Start with unique_ptr and shared_ptr. (intentionally exclude weak_ptr) | ||
static bool isSmartOwningPtrType(QualType QT) { | ||
QT = QT->getCanonicalTypeUnqualified(); | ||
|
||
auto isSmartPtrName = [](StringRef Name) { | ||
return Name == "unique_ptr" || Name == "shared_ptr"; | ||
}; | ||
|
||
// First try TemplateSpecializationType (for std smart pointers) | ||
if (const auto *TST = QT->getAs<TemplateSpecializationType>()) { | ||
const TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl(); | ||
if (!TD) | ||
return false; | ||
|
||
const auto *ND = dyn_cast_or_null<NamedDecl>(TD->getTemplatedDecl()); | ||
if (!ND) | ||
return false; | ||
|
||
// Check if it's in std namespace | ||
if (!isWithinStdNamespace(ND)) | ||
return false; | ||
|
||
return isSmartPtrName(ND->getName()); | ||
} | ||
|
||
// Also try RecordType (for custom smart pointer implementations) | ||
if (const auto *RD = QT->getAsCXXRecordDecl()) { | ||
// Accept any custom unique_ptr or shared_ptr implementation | ||
return (isSmartPtrName(RD->getName())); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
static bool hasSmartPtrField(const CXXRecordDecl *CRD) { | ||
// Check direct fields | ||
if (llvm::any_of(CRD->fields(), [](const FieldDecl *FD) { | ||
return isSmartOwningPtrType(FD->getType()); | ||
})) | ||
return true; | ||
|
||
// Check fields from base classes | ||
for (const CXXBaseSpecifier &Base : CRD->bases()) { | ||
if (const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl()) { | ||
if (hasSmartPtrField(BaseDecl)) | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
static bool isRvalueByValueRecord(const Expr *AE) { | ||
if (AE->isGLValue()) | ||
return false; | ||
|
||
QualType T = AE->getType(); | ||
if (!T->isRecordType() || T->isReferenceType()) | ||
return false; | ||
|
||
// Accept common temp/construct forms but don't overfit. | ||
return isa<CXXTemporaryObjectExpr, MaterializeTemporaryExpr, CXXConstructExpr, | ||
InitListExpr, ImplicitCastExpr, CXXBindTemporaryExpr>(AE); | ||
} | ||
|
||
static bool isRvalueByValueRecordWithSmartPtr(const Expr *AE) { | ||
if (!isRvalueByValueRecord(AE)) | ||
return false; | ||
|
||
const auto *CRD = AE->getType()->getAsCXXRecordDecl(); | ||
return CRD && hasSmartPtrField(CRD); | ||
} | ||
|
||
static ProgramStateRef escapeAllAllocatedSymbols(ProgramStateRef State) { | ||
RegionStateTy RS = State->get<RegionState>(); | ||
ProgramStateRef NewState = State; | ||
for (auto [Sym, RefSt] : RS) { | ||
if (RefSt.isAllocated() || RefSt.isAllocatedOfSizeZero()) { | ||
NewState = NewState->set<RegionState>(Sym, RefState::getEscaped(&RefSt)); | ||
} | ||
} | ||
return NewState; | ||
} | ||
|
||
static void collectDirectSmartOwningPtrFieldRegions( | ||
const MemRegion *Base, QualType RecQT, CheckerContext &C, | ||
SmallVectorImpl<const MemRegion *> &Out) { | ||
if (!Base) | ||
return; | ||
const auto *CRD = RecQT->getAsCXXRecordDecl(); | ||
if (!CRD) | ||
return; | ||
|
||
// Collect direct fields | ||
for (const FieldDecl *FD : CRD->fields()) { | ||
if (!isSmartOwningPtrType(FD->getType())) | ||
continue; | ||
SVal L = C.getState()->getLValue(FD, loc::MemRegionVal(Base)); | ||
if (const MemRegion *FR = L.getAsRegion()) | ||
Out.push_back(FR); | ||
} | ||
Comment on lines
+3214
to
+3220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we iterate fields. What about fields from base classes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added base class field support: |
||
|
||
// Collect fields from base classes | ||
for (const CXXBaseSpecifier &BaseSpec : CRD->bases()) { | ||
if (const CXXRecordDecl *BaseDecl = | ||
BaseSpec.getType()->getAsCXXRecordDecl()) { | ||
// Get the base class region | ||
SVal BaseL = C.getState()->getLValue(BaseDecl, Base->getAs<SubRegion>(), | ||
BaseSpec.isVirtual()); | ||
if (const MemRegion *BaseRegion = BaseL.getAsRegion()) { | ||
// Recursively collect fields from this base class | ||
collectDirectSmartOwningPtrFieldRegions(BaseRegion, BaseSpec.getType(), | ||
C, Out); | ||
} | ||
} | ||
} | ||
} | ||
|
||
void MallocChecker::checkPostCall(const CallEvent &Call, | ||
CheckerContext &C) const { | ||
// Keep existing post-call handlers. | ||
if (const auto *PostFN = PostFnMap.lookup(Call)) { | ||
(*PostFN)(this, C.getState(), Call, C); | ||
return; | ||
} | ||
|
||
SmallVector<const MemRegion *, 8> SmartPtrFieldRoots; | ||
ProgramStateRef State = C.getState(); | ||
|
||
for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { | ||
const Expr *AE = Call.getArgExpr(I); | ||
if (!AE) | ||
continue; | ||
AE = AE->IgnoreParenImpCasts(); | ||
|
||
if (!isRvalueByValueRecordWithSmartPtr(AE)) | ||
continue; | ||
|
||
// Find a region for the argument. | ||
SVal ArgVal = Call.getArgSVal(I); | ||
const MemRegion *ArgRegion = ArgVal.getAsRegion(); | ||
if (!ArgRegion) { | ||
// Fallback: if we have a by-value record with smart pointer fields but no | ||
// region, mark all allocated symbols as escaped | ||
State = escapeAllAllocatedSymbols(State); | ||
continue; | ||
} | ||
|
||
// Push direct smart owning pointer field regions only (precise root set). | ||
collectDirectSmartOwningPtrFieldRegions(ArgRegion, AE->getType(), C, | ||
SmartPtrFieldRoots); | ||
} | ||
|
||
// Escape only from those field roots | ||
if (!SmartPtrFieldRoots.empty()) { | ||
State = EscapeTrackedCallback::EscapeTrackedRegionsReachableFrom( | ||
SmartPtrFieldRoots, State); | ||
} | ||
|
||
// Apply state changes - addTransition will check if State differs | ||
// from current state | ||
C.addTransition(State); | ||
} | ||
|
||
void MallocChecker::checkPreCall(const CallEvent &Call, | ||
|
@@ -3194,7 +3398,6 @@ void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S, | |
if (!Sym) | ||
// If we are returning a field of the allocated struct or an array element, | ||
// the callee could still free the memory. | ||
// TODO: This logic should be a part of generic symbol escape callback. | ||
if (const MemRegion *MR = RetVal.getAsRegion()) | ||
if (isa<FieldRegion, ElementRegion>(MR)) | ||
if (const SymbolicRegion *BMR = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.