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

[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/214823.diff


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp (+74) 
- (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h (+5) 
- (modified) clang/test/Analysis/lifetime-bound.cpp (+77-16) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
index 2fab20b199f01..882c90d10d72a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp
@@ -105,6 +105,68 @@ std::string lifetime_modeling::getRegionName(const MemRegion *Reg) {
   return "the region";
 }
 
+namespace clang::ento::lifetime_modeling {
+
+static SmallVector<const MemRegion *, 4>
+getRegionsFromAggrVal(nonloc::LazyCompoundVal LCV, CheckerContext &C) {
+  SmallVector<const MemRegion *, 4> Reg;
+
+  const TypedValueRegion *LCVRegion = LCV.getRegion();
+  QualType T = LCVRegion->getValueType();
+  MemRegionManager &MemMgr = C.getSValBuilder().getRegionManager();
+  StoreManager &StoreMgr = C.getState()->getStateManager().getStoreManager();
+
+  // FIXME: getAsRecordDecl() also includes unions which need different
+  // handling. Reading a binding for every member of a union may produce regions
+  // that are not actually live.
+  if (const RecordDecl *RD = T->getAsRecordDecl()) {
+    RD = RD->getDefinition();
+    if (!RD)
+      return Reg;
+
+    for (const auto *FD : RD->fields()) {
+      // Unnamed bitfields in a record are not relevant for the analysis
+      // so the checker should skip them and just continue.
+      // CallAndMessageChecker has the same logic.
+      if (FD->isUnnamedBitField())
+        continue;
+
+      const FieldRegion *FR = MemMgr.getFieldRegion(FD, LCVRegion);
+      SVal V = StoreMgr.getBinding(LCV.getStore(), loc::MemRegionVal(FR));
+      if (const MemRegion *R = V.getAsRegion())
+        Reg.push_back(R);
+    }
+  }
+  return Reg;
+}
+
+static SmallVector<const MemRegion *, 4>
+getRegionsFromAggrVal(nonloc::CompoundVal CV, CheckerContext &C) {
+  SmallVector<const MemRegion *, 4> Reg;
+  for (SVal CVVal : CV) {
+    if (const MemRegion *CVReg = CVVal.getAsRegion())
+      Reg.push_back(CVReg);
+  }
+  return Reg;
+}
+
+} // namespace clang::ento::lifetime_modeling
+
+// FIXME: Retrieving the MemRegions of nested struct fields, base subobjects are
+// not yet supported. If a field of the aggregate is an aggregate (nested
+// structs) then no region is extracted for it. Handling it can be done by
+// recursing with non-aggregate fields as the base case.
+SmallVector<const MemRegion *, 4>
+lifetime_modeling::getRegionsFromAggrVal(SVal Val, CheckerContext &C) {
+  if (auto LCV = Val.getAs<nonloc::LazyCompoundVal>())
+    return getRegionsFromAggrVal(*LCV, C);
+
+  if (auto CV = Val.getAs<nonloc::CompoundVal>())
+    return getRegionsFromAggrVal(*CV, C);
+
+  return {};
+}
+
 void LifetimeModeling::checkPostCall(const CallEvent &Call,
                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
@@ -118,6 +180,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 +242,12 @@ 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..3be3e829f40da 100644
--- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h
@@ -22,6 +22,11 @@ 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..749f264fe53eb 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,80 @@ void no_dangling_by_value_argument() {
   // The returned reference does not dangle.
   takes_by_value(BoundToSelf());
 }
+
+struct IntPtr {
+  int *p;
+};
+
+IntPtr makeView(int &x [[clang::lifetimebound]]) { return IntPtr{&x}; }
+
+IntPtr 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    {{Lifetime of 'x' ended here}}
+  // expected-note at -3    {{Value's lifetime bound to the lifetime of 'x' here}}
+  // expected-warning at -4 {{Address of stack memory associated with local variable 'x' returned to caller}}
+  // expected-note at -5    {{Address of stack memory associated with local variable 'x' returned to caller}}
+  // expected-warning at -6 {{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    {{Lifetime of 'local' ended here}}
+  // expected-note at -3    {{Value's lifetime bound to the lifetime of 'local' here}}
+  // expected-warning at -4 {{Address of stack memory associated with local variable 'local' returned to caller}}
+  // expected-note at -5    {{Address of stack memory associated with local variable 'local' returned to caller}}
+  // expected-warning at -6 {{address of stack memory associated with local variable 'local' returned}}
+}
+
+struct NestedIntPtr {
+  IntPtr inner;
+  int *q;
+};
+
+NestedIntPtr makeNested(int &x [[clang::lifetimebound]]) {
+  return NestedIntPtr{IntPtr{&x}};
+}
+
+// FIXME: Nested structs are not yet handled by getRegionsFromAggrVal,
+// that is why this dangling pointer is not yet detected.
+NestedIntPtr 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}}
+}
+
+struct IntPtrArr {
+  int *arr[4];
+};
+
+IntPtrArr makeIntPtrArr(int &x [[clang::lifetimebound]]) {
+  return IntPtrArr{{&x, &global_v}};
+}
+
+// FIXME: Array fields are not split into their individual elements by
+// getRegionsFromAggrVal, that is why this dangling pointer is not yet
+// detected.
+IntPtrArr return_array_field_not_yet_detected() {
+  int z = 5;
+  return makeIntPtrArr(z);
+  // expected-warning at -1 {{Address of stack memory associated with local variable 'z' returned to caller}}
+  // expected-note at -2    {{Address of stack memory associated with local variable 'z' returned to caller}}
+  // expected-warning at -3 {{address of stack memory associated with local variable 'z' returned}}
+}

``````````

</details>


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