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@-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@-1 {{Returning value bound to 'x' that will go out of scope}} + // expected-note@-2 {{Lifetime of 'x' ended here}} + // expected-note@-3 {{Value's lifetime bound to the lifetime of 'x' here}} + // expected-warning@-4 {{Address of stack memory associated with local variable 'x' returned to caller}} + // expected-note@-5 {{Address of stack memory associated with local variable 'x' returned to caller}} + // expected-warning@-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@-1 {{Returning value bound to 'local' that will go out of scope}} + // expected-note@-2 {{Lifetime of 'local' ended here}} + // expected-note@-3 {{Value's lifetime bound to the lifetime of 'local' here}} + // expected-warning@-4 {{Address of stack memory associated with local variable 'local' returned to caller}} + // expected-note@-5 {{Address of stack memory associated with local variable 'local' returned to caller}} + // expected-warning@-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@-1 {{Address of stack memory associated with local variable 'y' returned to caller}} + // expected-note@-2 {{Address of stack memory associated with local variable 'y' returned to caller}} + // expected-warning@-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@-1 {{Address of stack memory associated with local variable 'z' returned to caller}} + // expected-note@-2 {{Address of stack memory associated with local variable 'z' returned to caller}} + // expected-warning@-3 {{address of stack memory associated with local variable 'z' returned}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/214823 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
