[clang] [analyzer] Add aggregate value tracking to the LifetimeModeling checker (PR #214589)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Benedek Kaibas (benedekaibas)

<details>
<summary>Changes</summary>

The `LifetimeModeling` checker currently does not analyze aggregate values (`CompoundVal`, `LazyCompoundVal`) which prevents tracking lifetime sources through structs passed or returned by value. This PR introduces a new mechanism which allows the modeling checker to track aggregate values through the `getRegionsFromAggrVal` function. 

Nested structs are currently not handled and that work will be done in a separate PR.

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


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp (+49) 
- (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h (+4) 
- (modified) clang/test/Analysis/lifetime-bound.cpp (+60-16) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
index 2fab20b199f01..f447f93207199 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
@@ -105,6 +105,44 @@ std::string lifetime_modeling::getRegionName(const MemRegion *Reg) {
   return "the region";
 }
 
+// FIXME: Retrieving the MemRegions of nested struct fields is not yet supported.
+SmallVector<const MemRegion *, 4>
+lifetime_modeling::getRegionsFromAggrVal(SVal Val, CheckerContext &C) {
+  SmallVector<const MemRegion *, 4> Reg;
+
+  if (auto LCV = Val.getAs<nonloc::LazyCompoundVal>()) {
+    const TypedValueRegion *LCVRegion = LCV->getRegion();
+    QualType T = LCVRegion->getValueType();
+    MemRegionManager &MemMgr = C.getSValBuilder().getRegionManager();
+    StoreManager &StoreMgr = C.getState()->getStateManager().getStoreManager();
+
+    if (const RecordType *RT = T->getAsStructureType()) {
+      const RecordDecl *RD = RT->getDecl()->getDefinition();
+      if (!RD)
+        return Reg;
+
+      for (const auto *I : RD->fields()) {
+        // Unnamed bitfields in a struct are not relevant for the analysis
+        // so the checker should skip them and jsut continue.
+        // CallAndMessageChecker has the same logic.
+        if (I->isUnnamedBitField())
+          continue;
+
+        const FieldRegion *FR = MemMgr.getFieldRegion(I, LCVRegion);
+        SVal V = StoreMgr.getBinding(LCV->getStore(), loc::MemRegionVal(FR));
+        if (const MemRegion *R = V.getAsRegion())
+          Reg.push_back(R);
+      }
+    }
+  } else if (auto CV = Val.getAs<nonloc::CompoundVal>()) {
+    for (SVal CVVal : *CV) {
+      if (const MemRegion *CVReg = CVVal.getAsRegion())
+        Reg.push_back(CVReg);
+    }
+  }
+  return Reg;
+}
+
 void LifetimeModeling::checkPostCall(const CallEvent &Call,
                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
@@ -118,6 +156,12 @@ void LifetimeModeling::checkPostCall(const CallEvent &Call,
     return;
 
   SVal RetVal = Call.getReturnValue();
+  SmallVector<const MemRegion *, 4> AggrRegs =
+      lifetime_modeling::getRegionsFromAggrVal(RetVal, C);
+
+  for (const MemRegion *I : AggrRegs) {
+    State = bindSource(State, RetVal, I);
+  }
 
   for (const ParmVarDecl *PVD : FD->parameters()) {
     if (PVD->hasAttr<LifetimeBoundAttr>()) {
@@ -174,6 +218,11 @@ void LifetimeModeling::checkDeadSymbols(SymbolReaper &SymReaper,
         S && SymReaper.isLive(S))
       continue;
 
+    if (llvm::any_of(
+            lifetime_modeling::getRegionsFromAggrVal(Val, C),
+            [&](const MemRegion *R) { return SymReaper.isLiveRegion(R); }))
+      continue;
+
     State = State->remove<LifetimeBoundMap>(Val);
   }
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h
index 8d6c8e4882d1c..5f1950944c878 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h
@@ -22,6 +22,10 @@ bool isBoundToLifetimeSource(ProgramStateRef State, SVal Val);
 /// Returns the descriptive name of the memory region or a placeholder if a
 /// descriptive name cannot be constructed for it.
 std::string getRegionName(const MemRegion *Reg);
+
+/// Returns the MemRegions the fields of an aggregate value (CompoundVal,
+/// LazyCompoundVal) point to.
+SmallVector<const MemRegion *, 4> getRegionsFromAggrVal(SVal Val, CheckerContext &C);
 } // namespace clang::ento::lifetime_modeling
 
 #endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_LIFETIMEMODELING_H
diff --git a/clang/test/Analysis/lifetime-bound.cpp b/clang/test/Analysis/lifetime-bound.cpp
index d29c37f639993..8a3ba8b3a6f10 100644
--- a/clang/test/Analysis/lifetime-bound.cpp
+++ b/clang/test/Analysis/lifetime-bound.cpp
@@ -155,22 +155,6 @@ void caller_nine() {
   // expected-note-re at -2    {{Origin '&SymRegion{{.*}}' bound to 'first_num', 'second_num'}}
 }
 
-struct View {
-  int *p;
-};
-View makeView(int &x [[clang::lifetimebound]]);
-
-void clang_analyzer_dumpLifetimeOriginsOf(View);
-
-void caller_view() {
-  int v = 42;
-  View w = makeView(v);
-  // FIXME: Currently none of the maps cover LazyCompoundVal.
-  clang_analyzer_dumpLifetimeOriginsOf(w); // no-warning
-}
-
-
-
 // These are the test cases for testing the correctness of the emitted warning from the UseAfterLifetimeEnd checker.
 
 // Return value bound to annotated param cases.
@@ -410,3 +394,63 @@ void no_dangling_by_value_argument() {
   // The returned reference does not dangle.
   takes_by_value(BoundToSelf());
 }
+
+struct F {
+  int *p;
+};
+
+F makeView(int &x [[clang::lifetimebound]]) { return F{&x}; }
+
+F whole_struct_return_lazycompoundval() {
+  int x = 5; // expected-note {{'x' initialized here}}
+  return makeView(x);
+  // expected-warning at -1 {{Returning value bound to 'x' that will go out of scope}}
+  // expected-note at -2    {{Returning value bound to 'x' that will go out of scope}}
+  // expected-warning at -3 {{Address of stack memory associated with local variable 'x' returned to caller}}
+  // expected-note at -4    {{Address of stack memory associated with local variable 'x' returned to caller}}
+  // expected-warning at -5 {{address of stack memory associated with local variable 'x' returned}}
+}
+
+struct PtrPair {
+  int *p;
+  int *q;
+};
+
+int global_v = 4;
+
+PtrPair makePair(int &x [[clang::lifetimebound]]) {
+  return PtrPair{&x, &global_v};
+}
+
+PtrPair return_pair_by_value() {
+  int local = 5; // expected-note {{'local' initialized here}}
+  return makePair(local);
+  // expected-warning at -1 {{Returning value bound to 'local' that will go out of scope}}
+  // expected-note at -2    {{Returning value bound to 'local' that will go out of scope}}
+  // expected-warning at -3 {{Address of stack memory associated with local variable 'local' returned to caller}}
+  // expected-note at -4    {{Address of stack memory associated with local variable 'local' returned to caller}}
+  // expected-warning at -5 {{address of stack memory associated with local variable 'local' returned}}
+}
+
+struct InnerS {
+  int *p;
+};
+
+struct OuterS {
+  InnerS inner;
+  int *q;
+};
+
+OuterS makeNested(int &x [[clang::lifetimebound]]) {
+  return OuterS{InnerS{&x}};
+}
+
+// FIXME: Nested structs are not yet handled by getRegionsFromAggrVal,
+// that is why this dangling pointer is not yet detected.
+OuterS nested_struct_return_not_yet_detected() {
+  int y = 5;
+  return makeNested(y);
+  // expected-warning at -1 {{Address of stack memory associated with local variable 'y' returned to caller}}
+  // expected-note at -2    {{Address of stack memory associated with local variable 'y' returned to caller}}
+  // expected-warning at -3 {{address of stack memory associated with local variable 'y' returned}}
+}

``````````

</details>


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