llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Benedek Kaibas (benedekaibas) <details> <summary>Changes</summary> `checkPostCall` in `LifetimeModeling` should only bind aggregate lifetime sources if the code is annotated with `[[clang::lifetimebound]]`. If the code is not annotated then it's `DanglingPtrDeref`'s job to bind aggregate lifetime sources for functions (which will be done in a separate PR). This PR is stacked on #<!-- -->214589. --- Full diff: https://github.com/llvm/llvm-project/pull/214737.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp (+63) - (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h (+5) - (modified) clang/test/Analysis/lifetime-bound.cpp (+94-16) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp index 2fab20b199f01..8de78960d1df2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp @@ -105,6 +105,56 @@ 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; +} + +static bool isAnnotated(const FunctionDecl *FD) { + for (const ParmVarDecl *PVD : FD->parameters()) { + if (PVD->hasAttr<LifetimeBoundAttr>()) + return true; + } + + if (lifetimes::implicitObjectParamIsLifetimeBound(FD)) + return true; + return false; +} + void LifetimeModeling::checkPostCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); @@ -119,6 +169,14 @@ void LifetimeModeling::checkPostCall(const CallEvent &Call, SVal RetVal = Call.getReturnValue(); + if (isAnnotated(FD)) { + 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>()) { unsigned Idx = PVD->getFunctionScopeIndex(); @@ -174,6 +232,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..d6cacfd73a4ef 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..5700cdb0e4a94 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,97 @@ 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@-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 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@-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 Hold { + int *ptr; +}; + +Hold retPtr(int &x) { + return Hold{&x}; +} +// Even though there is a lifetime error in the function +// UseAfterLifetimeEnd should not emit a warning for this +// case since there is no annotation present in the code. +// The warning present in the test comes from core.StackAddressEscape +// checker. +Hold return_by_val_no_ann() { + int num = 4; + return retPtr(num); + // expected-warning@-1 {{Address of stack memory associated with local variable 'num' returned to caller}} + // expected-note@-2 {{Address of stack memory associated with local variable 'num' returned to caller}} +} + +int *unwrap(Hold i [[clang::lifetimebound]]) { return i.ptr; } + +// FIXME: If an annotated argument is a by-value struct then +// Arg.getAsRegion() returns null for CompoundVal/LazyCompoundVal +// and the dangling pointer will not be detected. +int *arg_aggregate_lifetimebound() { + int local_num = 5; + Hold h{&local_num}; + return unwrap(h); + // expected-warning@-1 {{Address of stack memory associated with local variable 'local_num' returned to caller}} + // expected-note@-2 {{Address of stack memory associated with local variable 'local_num' returned to caller}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/214737 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
