llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) <details> <summary>Changes</summary> Fix handling of reference-typed DeclRefExpr in lifetime analysis Fixes https://github.com/llvm/llvm-project/issues/176399 This PR fixes a bug in the lifetime analysis where reference-typed DeclRefExpr nodes were incorrectly handled. The analysis was incorrectly removing the outer layer of origin for reference types, which led to missing some dangling reference warnings. The fix adds a check to only remove the outer layer of origin when the declaration is not a reference type. --- Full diff: https://github.com/llvm/llvm-project/pull/176728.diff 4 Files Affected: - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+4-3) - (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+2-1) - (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+13-5) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+23) ``````````diff diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 54e19b58bd7d5..5d1afd15c795d 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -589,9 +589,10 @@ void FactsGenerator::handleUse(const DeclRefExpr *DRE) { OriginList *List = getOriginsList(*DRE); if (!List) return; - // Remove the outer layer of origin which borrows from the decl directly. This - // is a use of the underlying decl. - List = getRValueOrigins(DRE, List); + // Remove the outer layer of origin which borrows from the decl directly + // (e.g., when this is not a reference). This is a use of the underlying decl. + if (!DRE->getDecl()->getType()->isReferenceType()) + List = getRValueOrigins(DRE, List); // Skip if there is no inner origin (e.g., when it is not a pointer type). if (!List) return; diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index 918547dfcec51..b47fe78bdf0fb 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -49,7 +49,8 @@ struct vector { template<typename InputIterator> vector(InputIterator first, InputIterator __last); - T &at(int n); + T & at(int n) &; + T && at(int n) &&; void push_back(const T&); void push_back(T&&); diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 7fdc493dbd17a..fac5eb68d1f50 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -280,13 +280,21 @@ std::string_view danglingRefToOptionalFromTemp4() { } void danglingReferenceFromTempOwner() { + int &&r = *std::optional<int>(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \ + // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} // FIXME: Detect this using the CFG-based lifetime analysis. // https://github.com/llvm/llvm-project/issues/175893 - int &&r = *std::optional<int>(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} int &&r2 = *std::optional<int>(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} + + // FIXME: Detect this using the CFG-based lifetime analysis. + // https://github.com/llvm/llvm-project/issues/175893 int &&r3 = std::optional<int>(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} - int &r4 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} - use(r, r2, r3, r4); + + const int &r4 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \ + // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} + int &&r5 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \ + // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} + use(r, r2, r3, r4, r5); // cfg-note 3 {{later used here}} std::string_view sv = *getTempOptStr(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \ // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} @@ -299,8 +307,8 @@ std::optional<std::vector<int>> getTempOptVec(); void testLoops() { for (auto i : getTempVec()) // ok ; - // FIXME: Detect this using the CFG-based lifetime analysis. - for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} + for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \ + // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} cfg-note {{later used here}} ; } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 0b1962b7cb651..655cdf64046f3 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1431,3 +1431,26 @@ void not_silenced_via_conditional(bool cond) { (void)v; // expected-note 2 {{later used here}} } } // namespace do_not_warn_on_std_move + +namespace reference_type_decl_ref_expr { +struct S { + S(); + ~S(); + const std::string& x() const [[clang::lifetimebound]]; +}; + +void test_temporary() { + const std::string& x = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} expected-note {{destroyed here}} + (void)x; // expected-note {{later used here}} +} + +void test_lifetime_extension_ok() { + const S& x = S(); + (void)x; +} + +const std::string& test_return() { + const std::string& x = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} expected-note {{destroyed here}} + return x; // expected-note {{later used here}} +} +} // namespace reference_type_decl_ref_expr `````````` </details> https://github.com/llvm/llvm-project/pull/176728 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
