[clang] [clang][analyzer] Fix state invalidation for modeled smart pointer me… (PR #229986)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Aaditya Agarwal (aaditya8979)

<details>
<summary>Changes</summary>

### Description
This PR resolves a false positive in `alpha.cplusplus.SmartPtr` where repeated calls to `std::unique_ptr::get()` on the same object returned identity-distinct symbols. This instability caused deterministic equivalence checks (e.g., `X == Y`) to incorrectly bifurcate into both `TRUE` and `FALSE` execution paths.

### Root Cause
`SmartPtrModeling` intercepts calls to smart pointer methods (`get()`, `release()`, `reset()`, `swap()`) via `evalCall`, establishing precise memory state transitions in the `TrackedRegionMap`. 

However, `ExprEngine::evalCall` runs an unconditional post-call `PSK_EscapeOutParameters` escape scan for every C++ method call on an object with a non-trivial destructor. Because `std::unique_ptr<T>` has a non-trivial destructor, its inner pointer is unconditionally marked as escaped after every modeled method call. 

This triggers a destructive chain:
`runCheckersForPointerEscape` → `checkRegionChanges` → `removeTrackedSubregions` → **`TrackedRegionMap` entry wiped.**

Consequently, the map entry established by the handler is destroyed before the next statement evaluates. A subsequent call to `P.get()` finds an empty cache and conjures a fresh, identity-distinct symbol (`conj_$1`), breaking symbol equivalence.

### The Fix
This patch introduces a surgical bypass in `SmartPtrModeling::checkRegionChanges`. 

`SmartPtrMethodHandlers` encodes exactly which `CallEvent`s the checker explicitly models. If `checkRegionChanges` is invoked for a call that matches one of our handlers, the handler has already produced the authoritative, deterministic memory state. The conservative region wipe is preempted, preserving the cached `TrackedRegionMap` state.

```cpp
if (Call && SmartPtrMethodHandlers.lookup(*Call))
  return State;

---
Full diff: https://github.com/llvm/llvm-project/pull/229986.diff


1 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp (+10) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index ad81e5076f931..de549377d6f77 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -573,6 +573,16 @@ ProgramStateRef SmartPtrModeling::checkRegionChanges(
     ArrayRef<const MemRegion *> ExplicitRegions,
     ArrayRef<const MemRegion *> Regions, const StackFrame *SF,
     const CallEvent *Call) const {
+  // Preempt conservative invalidation. If the invalidation is triggered by a
+  // modeled smart pointer method (e.g., get(), release(), swap()), the handler
+  // has already executed the precise, deterministic memory transitions via
+  // TrackedRegionMap. The ExprEngine's post-call PSK_EscapeOutParameters scan
+  // fires unconditionally for any object with a non-trivial destructor, which
+  // causes removeTrackedSubregions to blindly wipe our established state. If
+  // we own the CallEvent, we trust the state our handler produced.
+  if (Call && SmartPtrMethodHandlers.lookup(*Call))
+    return State;
+
   TrackedRegionMapTy RegionMap = State->get<TrackedRegionMap>();
   TrackedRegionMapTy::Factory &RegionMapFactory =
       State->get_context<TrackedRegionMap>();

``````````

</details>


https://github.com/llvm/llvm-project/pull/229986