diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 369d6194dbb65..8550106a058ae 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -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" @@ -421,6 +422,13 @@ class MallocChecker void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; bool evalCall(const CallEvent &Call, CheckerContext &C) const; + + ProgramStateRef + handleSmartPointerConstructorArguments(const CallEvent &Call, + ProgramStateRef State) const; + ProgramStateRef handleSmartPointerRelatedCalls(const CallEvent &Call, + CheckerContext &C, + ProgramStateRef State) const; void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const; void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const; @@ -1096,6 +1104,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)) {} + +public: + bool VisitSymbol(SymbolRef Sym) override { + if (const RefState *RS = State->get(Sym)) { + if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) { + State = State->set(Sym, RefState::getEscaped(RS)); + } + } + return true; + } + + /// Escape tracked regions reachable from the given roots. + static ProgramStateRef + EscapeTrackedRegionsReachableFrom(ArrayRef Roots, + ProgramStateRef State) { + if (Roots.empty()) + return State; + + // scanReachableSymbols is expensive, so we use a single visitor for all + // roots + SmallVector 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 +3124,260 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, C.addTransition(state->set(RS), N); } +// Allowlist of owning smart pointers we want to recognize. +// Start with unique_ptr and shared_ptr; weak_ptr is excluded intentionally +// because it does not own the pointee. +static bool isSmartPtrName(StringRef Name) { + return Name == "unique_ptr" || Name == "shared_ptr"; +} + +// Check if a type is a smart owning pointer type. +static bool isSmartPtrType(QualType QT) { + QT = QT->getCanonicalTypeUnqualified(); + + if (const auto *TST = QT->getAs()) { + const TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl(); + if (!TD) + return false; + + const auto *ND = dyn_cast_or_null(TD->getTemplatedDecl()); + if (!ND) + return false; + + // For broader coverage we recognize all template classes with names that + // match the allowlist even if they are not declared in namespace 'std'. + return isSmartPtrName(ND->getName()); + } + + return false; +} + +/// Helper struct for collecting smart owning pointer field regions. +/// This allows both hasSmartPtrField and +/// collectSmartPtrFieldRegions to share the same traversal logic, +/// ensuring consistency. +struct FieldConsumer { + const MemRegion *Reg; + CheckerContext *C; + llvm::SmallPtrSetImpl *Out; + + FieldConsumer(const MemRegion *Reg, CheckerContext &C, + llvm::SmallPtrSetImpl &Out) + : Reg(Reg), C(&C), Out(&Out) {} + + void consume(const FieldDecl *FD) { + SVal L = C->getState()->getLValue(FD, loc::MemRegionVal(Reg)); + if (const MemRegion *FR = L.getAsRegion()) + Out->insert(FR); + } + + std::optional switchToBase(const CXXRecordDecl *BaseDecl, + bool IsVirtual) { + // Get the base class region + SVal BaseL = + C->getState()->getLValue(BaseDecl, Reg->getAs(), IsVirtual); + if (const MemRegion *BaseObjRegion = BaseL.getAsRegion()) { + // Return a consumer for the base class + return FieldConsumer{BaseObjRegion, *C, *Out}; + } + return std::nullopt; + } +}; + +/// Check if a record type has smart owning pointer fields (directly or in base +/// classes). When FC is provided, also collect the field regions. +/// +/// This function has dual behavior: +/// - When FC is nullopt: Returns true if smart pointer fields are found +/// - When FC is provided: Always returns false, but collects field regions +/// as a side effect through the FieldConsumer +/// +/// Note: When FC is provided, the return value should be ignored since the +/// function performs full traversal for collection and always returns false +/// to avoid early termination. +static bool hasSmartPtrField(const CXXRecordDecl *CRD, + std::optional FC = std::nullopt) { + // Check direct fields + for (const FieldDecl *FD : CRD->fields()) { + if (isSmartPtrType(FD->getType())) { + if (!FC) + return true; + FC->consume(FD); + } + } + + // Check fields from base classes + for (const CXXBaseSpecifier &BaseSpec : CRD->bases()) { + if (const CXXRecordDecl *BaseDecl = + BaseSpec.getType()->getAsCXXRecordDecl()) { + std::optional NewFC; + if (FC) { + NewFC = FC->switchToBase(BaseDecl, BaseSpec.isVirtual()); + if (!NewFC) + continue; + } + bool Found = hasSmartPtrField(BaseDecl, NewFC); + if (Found && !FC) + return true; + } + } + return false; +} + +/// Check if an expression is an rvalue record type passed by value. +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(AE); +} + +/// Check if an expression is an rvalue record with smart owning pointer fields +/// passed by value. +static bool isRvalueByValueRecordWithSmartPtr(const Expr *AE) { + if (!isRvalueByValueRecord(AE)) + return false; + + const auto *CRD = AE->getType()->getAsCXXRecordDecl(); + return CRD && hasSmartPtrField(CRD); +} + +/// Check if a CXXRecordDecl has a name matching recognized smart pointer names. +static bool isSmartPtrRecord(const CXXRecordDecl *RD) { + if (!RD) + return false; + + // Check the record name directly and accept both std and custom smart pointer + // implementations for broader coverage + return isSmartPtrName(RD->getName()); +} + +/// Check if a call is a constructor of a smart owning pointer class that +/// accepts pointer parameters. +static bool isSmartPtrCall(const CallEvent &Call) { + // Only check for smart pointer constructor calls + const auto *CD = dyn_cast_or_null(Call.getDecl()); + if (!CD) + return false; + + const auto *RD = CD->getParent(); + if (!isSmartPtrRecord(RD)) + return false; + + // Check if constructor takes a pointer parameter + for (const auto *Param : CD->parameters()) { + QualType ParamType = Param->getType(); + if (ParamType->isPointerType() && !ParamType->isFunctionPointerType() && + !ParamType->isVoidPointerType()) { + return true; + } + } + + return false; +} + +/// Collect memory regions of smart owning pointer fields from a record type +/// (including fields from base classes). +static void +collectSmartPtrFieldRegions(const MemRegion *Reg, QualType RecQT, + CheckerContext &C, + llvm::SmallPtrSetImpl &Out) { + if (!Reg) + return; + + const auto *CRD = RecQT->getAsCXXRecordDecl(); + if (!CRD) + return; + + FieldConsumer FC{Reg, C, Out}; + hasSmartPtrField(CRD, FC); +} + +/// Handle smart pointer constructor calls by escaping allocated symbols +/// that are passed as pointer arguments to the constructor. +ProgramStateRef MallocChecker::handleSmartPointerConstructorArguments( + const CallEvent &Call, ProgramStateRef State) const { + const auto *CD = cast(Call.getDecl()); + for (unsigned I = 0, E = std::min(Call.getNumArgs(), CD->getNumParams()); + I != E; ++I) { + const Expr *ArgExpr = Call.getArgExpr(I); + if (!ArgExpr) + continue; + + QualType ParamType = CD->getParamDecl(I)->getType(); + if (ParamType->isPointerType() && !ParamType->isFunctionPointerType() && + !ParamType->isVoidPointerType()) { + // This argument is a pointer being passed to smart pointer constructor + SVal ArgVal = Call.getArgSVal(I); + SymbolRef Sym = ArgVal.getAsSymbol(); + if (Sym && State->contains(Sym)) { + const RefState *RS = State->get(Sym); + if (RS && (RS->isAllocated() || RS->isAllocatedOfSizeZero())) { + State = State->set(Sym, RefState::getEscaped(RS)); + } + } + } + } + return State; +} + +/// Handle all smart pointer related processing in function calls. +/// This includes both direct smart pointer constructor calls and by-value +/// arguments containing smart pointer fields. +ProgramStateRef MallocChecker::handleSmartPointerRelatedCalls( + const CallEvent &Call, CheckerContext &C, ProgramStateRef State) const { + + // Handle direct smart pointer constructor calls first + if (isSmartPtrCall(Call)) { + return handleSmartPointerConstructorArguments(Call, State); + } + + // Handle smart pointer fields in by-value record arguments + llvm::SmallPtrSet SmartPtrFieldRoots; + 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(); + // Collect direct smart owning pointer field regions + collectSmartPtrFieldRegions(ArgRegion, AE->getType(), C, + SmartPtrFieldRoots); + } + + // Escape symbols reachable from smart pointer fields + if (!SmartPtrFieldRoots.empty()) { + SmallVector SmartPtrFieldRootsVec( + SmartPtrFieldRoots.begin(), SmartPtrFieldRoots.end()); + State = EscapeTrackedCallback::EscapeTrackedRegionsReachableFrom( + SmartPtrFieldRootsVec, State); + } + + return State; +} + void MallocChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { + // Handle existing post-call handlers first if (const auto *PostFN = PostFnMap.lookup(Call)) { (*PostFN)(this, C.getState(), Call, C); - return; + return; // Post-handler already called addTransition, we're done } + + // Handle smart pointer related processing only if no post-handler was called + C.addTransition(handleSmartPointerRelatedCalls(Call, C, C.getState())); } void MallocChecker::checkPreCall(const CallEvent &Call, @@ -3194,7 +3498,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(MR)) if (const SymbolicRegion *BMR = diff --git a/clang/test/Analysis/NewDeleteLeaks-PR60896.cpp b/clang/test/Analysis/NewDeleteLeaks-PR60896.cpp new file mode 100644 index 0000000000000..00dbbf2ec3c7e --- /dev/null +++ b/clang/test/Analysis/NewDeleteLeaks-PR60896.cpp @@ -0,0 +1,252 @@ +// RUN: %clang_analyze_cc1 -verify -analyzer-output=text %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus \ +// RUN: -analyzer-checker=unix + +#include "Inputs/system-header-simulator-for-malloc.h" + +//===----------------------------------------------------------------------===// +// unique_ptr test cases +//===----------------------------------------------------------------------===// +namespace unique_ptr_tests { + +// Custom unique_ptr implementation for testing +template +struct unique_ptr { + T* ptr; + unique_ptr(T* p) : ptr(p) {} + ~unique_ptr() { + // This destructor intentionally doesn't delete 'ptr' to validate that the + // heuristic trusts that smart pointers (based on their class name) will + // release the pointee even if it doesn't understand their destructor. + } + unique_ptr(unique_ptr&& other) : ptr(other.ptr) { other.ptr = nullptr; } + T* get() const { return ptr; } +}; + +template +unique_ptr make_unique(Args&&... args) { + return unique_ptr(new T(args...)); +} + +// Test 1: Check that we report leaks for malloc when passing smart pointers +void add_unique_ptr(unique_ptr ptr) { + // The unique_ptr destructor will be called when ptr goes out of scope +} + +void test_malloc_with_smart_ptr() { + void *ptr = malloc(4); // expected-note {{Memory is allocated}} + + add_unique_ptr(make_unique(1)); + (void)ptr; + // expected-warning@+1 {{Potential leak of memory pointed to by 'ptr'}} expected-note@+1 {{Potential leak of memory pointed to by 'ptr'}} +} + +// Test 2: Check that we don't report leaks for unique_ptr in temporary objects +struct Foo { + unique_ptr i; +}; + +void add_foo(Foo foo) { + // The unique_ptr destructor will be called when foo goes out of scope +} + +void test_temporary_object() { + // No warning should be emitted for this - the memory is managed by unique_ptr + // in the temporary Foo object, which will properly clean up the memory + add_foo({make_unique(1)}); +} + +// Test 3: Check that we don't report leaks for smart pointers in base class fields +struct Base { + unique_ptr base_ptr; + Base() : base_ptr(nullptr) {} + Base(unique_ptr&& ptr) : base_ptr(static_cast&&>(ptr)) {} +}; + +struct Derived : public Base { + int derived_field; + Derived() : Base(), derived_field(0) {} + Derived(unique_ptr&& ptr, int field) : Base(static_cast&&>(ptr)), derived_field(field) {} +}; + +void add_derived(Derived derived) { + // The unique_ptr destructor will be called when derived goes out of scope + // This should include the base_ptr field from the base class +} + +void test_base_class_smart_ptr() { + // No warning should be emitted for this - the memory is managed by unique_ptr + // in the base class field of the temporary Derived object + add_derived(Derived(make_unique(1), 42)); +} + +// Test 4: Check that we don't report leaks for multiple owning arguments +struct SinglePtr { + unique_ptr ptr; + SinglePtr(unique_ptr&& p) : ptr(static_cast&&>(p)) {} +}; + +struct MultiPtr { + unique_ptr ptr1; + unique_ptr ptr2; + unique_ptr ptr3; + + MultiPtr(unique_ptr&& p1, unique_ptr&& p2, unique_ptr&& p3) + : ptr1(static_cast&&>(p1)) + , ptr2(static_cast&&>(p2)) + , ptr3(static_cast&&>(p3)) {} +}; + +void addMultiple(SinglePtr single, MultiPtr multi) { + // All unique_ptr destructors will be called when the objects go out of scope + // This tests handling of multiple by-value arguments with smart pointer fields +} + +void test_multiple_owning_args() { + // No warning should be emitted - all memory is properly managed by unique_ptr + // in the temporary objects, which will properly clean up the memory + addMultiple( + SinglePtr(make_unique(1)), + MultiPtr(make_unique(2), make_unique(3), make_unique(4)) + ); +} + +// Test 5: Check that we DO report leaks for raw pointers in mixed ownership scenarios +struct MixedOwnership { + unique_ptr smart_ptr; // Should NOT leak (smart pointer managed) + int *raw_ptr; // Should leak (raw pointer) + + MixedOwnership() : smart_ptr(make_unique(1)), raw_ptr(new int(42)) {} // expected-note {{Memory is allocated}} +}; + +void consume(MixedOwnership obj) { + // The unique_ptr destructor will be called when obj goes out of scope + // But raw_ptr will leak! +} + +void test_mixed_ownership() { + // This should report a leak for raw_ptr but not for smart_ptr + consume(MixedOwnership()); // expected-note {{Calling default constructor for 'MixedOwnership'}} expected-note {{Returning from default constructor for 'MixedOwnership'}} +} // expected-warning {{Potential memory leak}} expected-note {{Potential memory leak}} + +// Test 6: Check that we handle direct smart pointer constructor calls correctly +void test_direct_constructor() { + // Direct constructor call - should not leak + int* raw_ptr = new int(42); + unique_ptr smart(raw_ptr); // This should escape the raw_ptr symbol + // No leak should be reported here since smart pointer takes ownership +} + +void test_mixed_direct_constructor() { + int* raw1 = new int(1); + int* raw2 = new int(2); // expected-note {{Memory is allocated}} + + unique_ptr smart(raw1); // This should escape raw1 + // raw2 should leak since it's not managed by any smart pointer + int x = *raw2; // expected-warning {{Potential leak of memory pointed to by 'raw2'}} expected-note {{Potential leak of memory pointed to by 'raw2'}} +} + +// Test 7: Multiple memory owning arguments - demonstrates addTransition API usage +void addMultipleOwningArgs( + unique_ptr ptr1, + unique_ptr ptr2, + unique_ptr ptr3 +) { + // All unique_ptr destructors will be called when arguments go out of scope + // This tests handling of multiple smart pointer parameters in a single call +} + +void test_multiple_memory_owning_arguments() { + // No warning should be emitted - all memory is properly managed by unique_ptr + // This test specifically exercises the addTransition API with multiple owning arguments + addMultipleOwningArgs( + make_unique(1), + make_unique(2), + make_unique(3) + ); +} + +} // namespace unique_ptr_tests + +//===----------------------------------------------------------------------===// +// Variadic constructor test cases +//===----------------------------------------------------------------------===// +namespace variadic_constructor_tests { + +// Variadic constructor - test for potential out-of-bounds access +// This is the only test in this namespace and tests a scenario where Call.getNumArgs() > CD->getNumParams() +// We use a synthetic unique_ptr here to activate the specific logic in the MallocChecker that will test out of bounds +template +struct unique_ptr { + T* ptr; + + // Constructor with ellipsis - can receive more arguments than parameters + unique_ptr(T* p, ...) : ptr(p) {} + + ~unique_ptr() { + // This destructor intentionally doesn't delete 'ptr' to validate that the + // heuristic trusts that smart pointers (based on their class name) will + // release the pointee even if it doesn't understand their destructor. + } +}; + +void process_variadic_smart_ptr(unique_ptr ptr) { + // Function body doesn't matter for this test +} + +void test_variadic_constructor_bounds() { + void *malloc_ptr = malloc(4); // expected-note {{Memory is allocated}} + + // This call creates a smart pointer with more arguments than formal parameters + // The constructor has 1 formal parameter (T* p) plus ellipsis, but we pass multiple args + // This should trigger the bounds checking issue in handleSmartPointerConstructorArguments + int* raw_ptr = new int(42); + process_variadic_smart_ptr(unique_ptr(raw_ptr, 1, 2, 3, 4, 5)); + + (void)malloc_ptr; +} // expected-warning {{Potential leak of memory pointed to by 'malloc_ptr'}} + // expected-note@-1 {{Potential leak of memory pointed to by 'malloc_ptr'}} + +} // namespace variadic_constructor_tests + +//===----------------------------------------------------------------------===// +// shared_ptr test cases +//===----------------------------------------------------------------------===// +namespace shared_ptr_tests { + +// Custom shared_ptr implementation for testing +template +struct shared_ptr { + T* ptr; + shared_ptr(T* p) : ptr(p) {} + ~shared_ptr() { + // This destructor intentionally doesn't delete 'ptr' to validate that the + // heuristic trusts that smart pointers (based on their class name) will + // release the pointee even if it doesn't understand their destructor. + } + shared_ptr(shared_ptr&& other) : ptr(other.ptr) { other.ptr = nullptr; } + T* get() const { return ptr; } +}; + +template +shared_ptr make_shared(Args&&... args) { + return shared_ptr(new T(args...)); +} + +// Test 1: Check that we don't report leaks for shared_ptr in temporary objects +struct Foo { + shared_ptr i; +}; + +void add_foo(Foo foo) { + // The shared_ptr destructor will be called when foo goes out of scope +} + +void test_temporary_object() { + // No warning should be emitted for this - the memory is managed by shared_ptr + // in the temporary Foo object, which will properly clean up the memory + add_foo({make_shared(1)}); +} + +} // namespace shared_ptr_tests