[analyzer] MallocChecker – Fix false positive leak for smart pointers in temporary objects

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -1122,6 +1122,7 @@ class EscapeTrackedCallback final : public SymbolVisitor {
 
   explicit EscapeTrackedCallback(ProgramStateRef S) : State(std::move(S)) {}
 
+public:
   bool VisitSymbol(SymbolRef Sym) override {
     if (const RefState *RS = State->get<RegionState>(Sym)) {
       if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) {
@@ -1131,7 +1132,6 @@ class EscapeTrackedCallback final : public SymbolVisitor {
     return true;
   }
 
-public:
   /// Escape tracked regions reachable from the given roots.
   static ProgramStateRef
   EscapeTrackedRegionsReachableFrom(ArrayRef<const MemRegion *> Roots,
@@ -3124,18 +3124,17 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   C.addTransition(state->set<RegionState>(RS), N);
 }
 
-// Helper function to check if a name is a recognized smart owning pointer name
-static bool isSmartOwningPtrName(StringRef Name) {
+// 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";
 }
 
-// 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) {
+// Check if a type is a smart owning pointer type.
+static bool isSmartPtrType(QualType QT) {
   QT = QT->getCanonicalTypeUnqualified();
 
-  // First try TemplateSpecializationType (for both std and custom smart
-  // pointers)
   if (const auto *TST = QT->getAs<TemplateSpecializationType>()) {
     const TemplateDecl *TD = TST->getTemplateName().getAsTemplateDecl();
     if (!TD)
@@ -3145,27 +3144,71 @@ static bool isSmartOwningPtrType(QualType QT) {
     if (!ND)
       return false;
 
-    // Accept both std and custom smart pointer implementations for broader
-    // coverage
-    return isSmartOwningPtrName(ND->getName());
+    // 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<const MemRegion *> *Out;
+
+  FieldConsumer(const MemRegion *Reg, CheckerContext &C,
+                llvm::SmallPtrSetImpl<const MemRegion *> &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<FieldConsumer> switchToBase(const CXXRecordDecl *BaseDecl,
+                                            bool IsVirtual) {
+    // Get the base class region
+    SVal BaseL =
+        C->getState()->getLValue(BaseDecl, Reg->getAs<SubRegion>(), 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).
-static bool hasSmartOwningPtrField(const CXXRecordDecl *CRD) {
+/// classes). When FC is provided, also collect the field regions.
+static bool hasSmartPtrField(const CXXRecordDecl *CRD,
+                             std::optional<FieldConsumer> FC = std::nullopt) {
   // Check direct fields
-  if (llvm::any_of(CRD->fields(), [](const FieldDecl *FD) {
-        return isSmartOwningPtrType(FD->getType());
-      }))
-    return true;
+  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 &Base : CRD->bases()) {
-    if (const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl()) {
-      if (hasSmartOwningPtrField(BaseDecl))
+  for (const CXXBaseSpecifier &BaseSpec : CRD->bases()) {
+    if (const CXXRecordDecl *BaseDecl =
+            BaseSpec.getType()->getAsCXXRecordDecl()) {
+      std::optional<FieldConsumer> NewFC;
+      if (FC) {
+        NewFC = FC->switchToBase(BaseDecl, BaseSpec.isVirtual());
+        if (!NewFC)
+          continue;
+      }
+      bool Found = hasSmartPtrField(BaseDecl, NewFC);
+      if (Found && !FC)
         return true;
     }
   }
@@ -3188,34 +3231,34 @@ static bool isRvalueByValueRecord(const Expr *AE) {
 
 /// Check if an expression is an rvalue record with smart owning pointer fields
 /// passed by value.
-static bool isRvalueByValueRecordWithSmartOwningPtr(const Expr *AE) {
+static bool isRvalueByValueRecordWithSmartPtr(const Expr *AE) {
   if (!isRvalueByValueRecord(AE))
     return false;
 
   const auto *CRD = AE->getType()->getAsCXXRecordDecl();
-  return CRD && hasSmartOwningPtrField(CRD);
+  return CRD && hasSmartPtrField(CRD);
 }
 
 /// Check if a CXXRecordDecl has a name matching recognized smart pointer names.
-static bool isSmartOwningPtrRecord(const CXXRecordDecl *RD) {
+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 isSmartOwningPtrName(RD->getName());
+  return isSmartPtrName(RD->getName());
 }
 
 /// Check if a call is a constructor of a smart owning pointer class that
 /// accepts pointer parameters.
-static bool isSmartOwningPtrCall(const CallEvent &Call) {
+static bool isSmartPtrCall(const CallEvent &Call) {
   // Only check for smart pointer constructor calls
   const auto *CD = dyn_cast_or_null<CXXConstructorDecl>(Call.getDecl());
   if (!CD)
     return false;
 
   const auto *RD = CD->getParent();
-  if (!isSmartOwningPtrRecord(RD))
+  if (!isSmartPtrRecord(RD))
     return false;
 
   // Check if constructor takes a pointer parameter
@@ -3230,38 +3273,21 @@ static bool isSmartOwningPtrCall(const CallEvent &Call) {
   return false;
 }
 
-static void collectSmartOwningPtrFieldRegions(
-    const MemRegion *Base, QualType RecQT, CheckerContext &C,
-    llvm::SmallPtrSetImpl<const MemRegion *> &Out) {
-  if (!Base)
+/// 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<const MemRegion *> &Out) {
+  if (!Reg)
     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.insert(FR);
-  }
-
-  // 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
-        collectSmartOwningPtrFieldRegions(BaseRegion, BaseSpec.getType(), C,
-                                          Out);
-      }
-    }
-  }
+  FieldConsumer FC{Reg, C, Out};
+  hasSmartPtrField(CRD, FC);
 }
 
 /// Handle smart pointer constructor calls by escaping allocated symbols
@@ -3299,7 +3325,7 @@ ProgramStateRef MallocChecker::handleSmartPointerRelatedCalls(
     const CallEvent &Call, CheckerContext &C, ProgramStateRef State) const {
 
   // Handle direct smart pointer constructor calls first
-  if (isSmartOwningPtrCall(Call)) {
+  if (isSmartPtrCall(Call)) {
     return handleSmartPointerConstructorArguments(Call, State);
   }
 
@@ -3311,15 +3337,15 @@ ProgramStateRef MallocChecker::handleSmartPointerRelatedCalls(
       continue;
     AE = AE->IgnoreParenImpCasts();
 
-    if (!isRvalueByValueRecordWithSmartOwningPtr(AE))
+    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
-    collectSmartOwningPtrFieldRegions(ArgRegion, AE->getType(), C,
-                                      SmartPtrFieldRoots);
+    collectSmartPtrFieldRegions(ArgRegion, AE->getType(), C,
+                                SmartPtrFieldRoots);
   }
 
   // Escape symbols reachable from smart pointer fields

clang/test/Analysis/NewDeleteLeaks-PR60896.cpp

@@ -1,8 +1,7 @@
 // RUN: %clang_analyze_cc1 -verify -analyzer-output=text %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus \
-// RUN:   -analyzer-checker=unix \
-// RUN:   -analyzer-checker=unix.Malloc
+// RUN:   -analyzer-checker=unix
 
 #include "Inputs/system-header-simulator-for-malloc.h"
 
@@ -16,7 +15,11 @@ template <typename T>
 struct unique_ptr {
   T* ptr;
   unique_ptr(T* p) : ptr(p) {}
-  ~unique_ptr() { delete ptr; }
+  ~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; }
 };
@@ -164,19 +167,31 @@ void test_multiple_memory_owning_arguments() {
   );
 }
 
-// Test 8: Variadic constructor - test for potential out-of-bounds access
-// This tests a scenario where Call.getNumArgs() > CD->getNumParams()
+} // 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 <typename T>
-struct VariadicSmartPtr {
+struct unique_ptr {
   T* ptr;
-  
+
   // Constructor with ellipsis - can receive more arguments than parameters  
-  VariadicSmartPtr(T* p, ...) : ptr(p) {}
-
-  ~VariadicSmartPtr() { delete 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.
+  }
 };
 
-void process_variadic_smart_ptr(VariadicSmartPtr<int> ptr) {
+void process_variadic_smart_ptr(unique_ptr<int> ptr) {
   // Function body doesn't matter for this test
 }
 
@@ -187,12 +202,12 @@ void test_variadic_constructor_bounds() {
   // 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(VariadicSmartPtr<int>(raw_ptr, 1, 2, 3, 4, 5));
+  process_variadic_smart_ptr(unique_ptr<int>(raw_ptr, 1, 2, 3, 4, 5));
 
   (void)malloc_ptr;
 } // expected-warning {{Potential leak of memory pointed to by 'malloc_ptr'}} expected-note {{Potential leak of memory pointed to by 'malloc_ptr'}}
 
-} // namespace unique_ptr_tests
+} // namespace variadic_constructor_tests
 
 //===----------------------------------------------------------------------===//
 // shared_ptr test cases
@@ -204,7 +219,11 @@ template <typename T>
 struct shared_ptr {
   T* ptr;
   shared_ptr(T* p) : ptr(p) {}
-  ~shared_ptr() { delete ptr; }
+  ~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; }
 };