llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety @llvm/pr-subscribers-clang-analysis Author: Utkarsh Saxena (usx95) <details> <summary>Changes</summary> Enhanced lifetime safety analysis to support `std::make_unique` by propagating `lifetimebound` attributes from constructor parameters to `make_unique` parameters. - Added special handling for `std::make_unique` in `inferLifetimeBoundAttribute()` to automatically propagate `lifetimebound` attributes from the constructed type's constructor parameters to the corresponding `make_unique` parameters - Extended GSL owner type handling in assignment operations within `FactsGenerator::VisitCXXOperatorCallExpr()` `std::make_unique` is a common factory function that forwards arguments to constructors. Without this enhancement, lifetime safety analysis couldn't detect when `make_unique` was being used to create objects with lifetimebound dependencies, leading to missed warnings about potential dangling references. This change ensures that lifetime safety analysis works seamlessly with modern C++ idioms using smart pointer factory functions. --- **Current Limitation**: Lifetimebound propagation only occurs when the constructor parameter is a reference type. This restriction avoids incorrect loan tracking when value types (pointers, view types like `string_view`) are passed through forwarding references to `make_unique`. However, this means some legitimate dangling scenarios involving value-type parameters are not detected. This limitation presents an opportunity to experiment with `clang::lifetimebound(2)` to distinguish between parameter categories and enable more precise tracking for forwarding references and multi-level pointers in general. --- Full diff: https://github.com/llvm/llvm-project/pull/191632.diff 9 Files Affected: - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+2-1) - (modified) clang/lib/Analysis/LifetimeSafety/Origins.cpp (+1) - (modified) clang/lib/Sema/SemaAttr.cpp (+55) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+2) - (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+3) - (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+11) - (modified) clang/test/Sema/warn-lifetime-safety-dangling-field.cpp (+16) - (modified) clang/test/Sema/warn-lifetime-safety-suggestions.cpp (+25-3) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+89-5) ``````````diff diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 82b890b57817e..69a64d08e0385 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -484,7 +484,8 @@ void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { hasOrigins(OCE->getArg(0)->getType())) { // Pointer-like types: assignment inherently propagates origins. QualType LHSTy = OCE->getArg(0)->getType(); - if (LHSTy->isPointerOrReferenceType() || isGslPointerType(LHSTy)) { + if (LHSTy->isPointerOrReferenceType() || isGslPointerType(LHSTy) || + isGslOwnerType(LHSTy)) { handleAssignment(OCE->getArg(0), OCE->getArg(1)); return; } diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index abf9890b08522..c0f98dffe8354 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -68,6 +68,7 @@ class LifetimeAnnotatedOriginTypeCollector } bool shouldVisitLambdaBody() const { return false; } + bool shouldVisitTemplateInstantiations() const { return true; } const llvm::SmallVector<QualType> &getCollectedTypes() const { return CollectedTypes; diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 7c79f954e6743..d173b1ba58d8e 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -250,6 +250,61 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { } return; } + + // Handle std::make_unique to propagate lifetimebound attributes from the + // constructed type's constructor to make_unique's parameters. + if (FD->isInStdNamespace() && FD->getDeclName().isIdentifier() && + FD->getName() == "make_unique") { + if (!FD->isFunctionTemplateSpecialization()) + return; + + const TemplateArgumentList *TAL = FD->getTemplateSpecializationArgs(); + if (!TAL || TAL->size() < 1) + return; + + // make_unique's first template argument is the type being constructed. + TemplateArgument TA = TAL->get(0); + if (TA.getKind() != TemplateArgument::Type) + return; + + QualType T = TA.getAsType(); + const auto *RD = T->getAsCXXRecordDecl(); + if (!RD) + return; + + // Find the constructor that matches make_unique's arguments. + for (const auto *Ctor : RD->ctors()) { + if (Ctor->getNumParams() != FD->getNumParams()) + continue; + + bool Compatible = true; + for (unsigned i = 0; i < Ctor->getNumParams(); ++i) { + QualType CtorParamType = Ctor->getParamDecl(i)->getType(); + QualType FDParamType = FD->getParamDecl(i)->getType(); + // Compare types ignoring references. + if (!Context.hasSameUnqualifiedType( + CtorParamType.getNonReferenceType(), + FDParamType.getNonReferenceType())) { + Compatible = false; + break; + } + } + + if (!Compatible) + continue; + // Propagate lifetimebound attributes only if the constructor parameter is + // a reference. This avoids incorrect loan tracking when a by-value view + // (like string_view) is passed by reference to make_unique. + for (unsigned i = 0; i < Ctor->getNumParams(); ++i) + if (Ctor->getParamDecl(i)->hasAttr<LifetimeBoundAttr>() && + Ctor->getParamDecl(i)->getType()->isReferenceType()) + FD->getParamDecl(i)->addAttr( + LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation())); + break; // Found matching constructor, done. + } + return; + } + if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) { const auto *CRD = CMD->getParent(); if (!CRD->isInStdNamespace() || !CRD->getIdentifier()) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 09c2482168ab7..954160d4112a5 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5473,6 +5473,8 @@ TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New, SemaRef.InstantiateAttrs(TemplateArgs, Definition, New, LateAttrs, StartingScope); + SemaRef.inferLifetimeBoundAttribute(New); + return false; } diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index d1e847d20cc50..67e6c0f18de5c 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -205,6 +205,9 @@ struct unique_ptr { T *get() const; }; +template<typename T, typename... Args> +unique_ptr<T> make_unique(Args&&... args); + template<typename T> struct optional { optional(); diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 0ed151b9db136..c352cca2023a6 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -176,6 +176,17 @@ void initLocalGslPtrWithTempOwner() { use(global2, p2); // cfg-note 2 {{later used here}} } +struct LifetimeBoundCtor { + LifetimeBoundCtor(const MyIntOwner& obj1 [[clang::lifetimebound]]); + LifetimeBoundCtor(std::string_view sv [[clang::lifetimebound]]); +}; + +auto lifetimebound_make_unique_single_param() { + return std::make_unique<LifetimeBoundCtor>(MyIntOwner{}); // expected-warning {{returning address of local temporary object}} \ + // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} +} + + struct Unannotated { typedef std::vector<int>::iterator iterator; diff --git a/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp b/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp index fa45458371012..e35a68ceae961 100644 --- a/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dangling-field.cpp @@ -184,3 +184,19 @@ struct IndirectEscape2 { p = s.data(); } }; + +namespace MakeUnique { +struct MyObj {}; + +struct LifetimeBoundCtor { + LifetimeBoundCtor(const MyObj& obj [[clang::lifetimebound]]); +}; + +struct HasUniquePtrField { + std::unique_ptr<LifetimeBoundCtor> field; // expected-note {{this field dangles}} + + void setWithParam(MyObj obj) { + field = std::make_unique<LifetimeBoundCtor>(obj); // expected-warning {{address of stack memory escapes to a field}} + } +}; +} // namespace MakeUnique diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 19c3251b9c296..5906170b0d146 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -1,8 +1,8 @@ // RUN: rm -rf %t // RUN: split-file %s %t -// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wlifetime-safety -Wno-dangling -I%t -verify %t/test_source.cpp -// RUN: %clang_cc1 -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wlifetime-safety -Wno-dangling -I%t -fixit %t/test_source.cpp -// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wno-dangling -I%t -Werror=lifetime-safety-suggestions %t/test_source.cpp +// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wlifetime-safety -Wno-dangling -I%S -I%t -verify %t/test_source.cpp +// RUN: %clang_cc1 -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wlifetime-safety -Wno-dangling -I%S -I%t -fixit %t/test_source.cpp +// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety-suggestions -Wno-dangling -I%S -I%t -Werror=lifetime-safety-suggestions %t/test_source.cpp View definition_before_header(View a); @@ -10,6 +10,8 @@ View definition_before_header(View a); #ifndef TEST_HEADER_H #define TEST_HEADER_H +#include "Inputs/lifetime-analysis.h" + struct View; struct [[gsl::Owner]] MyObj { @@ -456,3 +458,23 @@ S forward(const MyObj &obj) { // expected-warning {{parameter in intra-TU functi } } // namespace track_origins_for_lifetimebound_record_type + +namespace make_unique_suggestion { + +struct LifetimeBoundCtor { + LifetimeBoundCtor(const MyObj& obj [[clang::lifetimebound]]); +}; + +std::unique_ptr<LifetimeBoundCtor> create_target(const MyObj& obj) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} + return std::make_unique<LifetimeBoundCtor>(obj); // expected-note {{param returned here}} +} + +void test_inference() { + std::unique_ptr<LifetimeBoundCtor> ptr; + { + MyObj obj; + ptr = create_target(obj); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} +} // namespace make_unique_suggestion diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 77d8e3370676d..e0147d69459f5 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -925,6 +925,7 @@ void lifetimebound_return_reference() { struct LifetimeBoundCtor { LifetimeBoundCtor(); LifetimeBoundCtor(const MyObj& obj [[clang::lifetimebound]]); + LifetimeBoundCtor(int* p [[clang::lifetimebound]]); }; void lifetimebound_ctor() { @@ -936,6 +937,91 @@ void lifetimebound_ctor() { (void)v; // expected-note {{later used here}} } +void lifetimebound_make_unique() { + std::unique_ptr<LifetimeBoundCtor> ptr; + { + MyObj obj; + ptr = std::make_unique<LifetimeBoundCtor>(obj); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + +void lifetimebound_make_unique_temp() { + std::unique_ptr<LifetimeBoundCtor> ptr = std::make_unique<LifetimeBoundCtor>(MyObj()); // expected-warning {{object whose reference is captured does not live long enough}} \ + // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + +void lifetimebound_make_unique_raw_ptr() { + std::unique_ptr<LifetimeBoundCtor> ptr; + int x = 0; + { + int* p = &x; + // FIXME: No warning expected with current implementation + ptr = std::make_unique<LifetimeBoundCtor>(p); + } + (void)ptr; +} + +void lifetimebound_make_unique_string_view_local() { + std::unique_ptr<LifetimeBoundCtor> ptr; + { + std::string s; + std::string_view sv(s); + // FIXME: No warning expected with current implementation because of reference mismatch + ptr = std::make_unique<LifetimeBoundCtor>(sv); + } + (void)ptr; +} + +struct MultiLifetimeBoundCtor { + MultiLifetimeBoundCtor(const MyObj& obj1 [[clang::lifetimebound]], const MyObj& obj2); + MultiLifetimeBoundCtor(const MyObj& obj1, const MyObj& obj2 [[clang::lifetimebound]], int); + MultiLifetimeBoundCtor(const MyObj& obj1 [[clang::lifetimebound]], const MyObj& obj2 [[clang::lifetimebound]], double); +}; + +void lifetimebound_make_unique_multi_params() { + std::unique_ptr<MultiLifetimeBoundCtor> ptr; + MyObj obj_long; + { + MyObj obj_short; + ptr = std::make_unique<MultiLifetimeBoundCtor>(obj_short, obj_long); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + +void lifetimebound_make_unique_multi_params2() { + std::unique_ptr<MultiLifetimeBoundCtor> ptr; + MyObj obj_long; + { + MyObj obj_short; + ptr = std::make_unique<MultiLifetimeBoundCtor>(obj_long, obj_short, 1); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + +void lifetimebound_make_unique_multi_params3_1() { + std::unique_ptr<MultiLifetimeBoundCtor> ptr; + MyObj obj_long; + { + MyObj obj_short; + ptr = std::make_unique<MultiLifetimeBoundCtor>(obj_short, obj_long, 1.0); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + +void lifetimebound_make_unique_multi_params3_2() { + std::unique_ptr<MultiLifetimeBoundCtor> ptr; + MyObj obj_long; + { + MyObj obj_short; + ptr = std::make_unique<MultiLifetimeBoundCtor>(obj_long, obj_short, 1.0); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ptr; // expected-note {{later used here}} +} + + + View lifetimebound_return_of_local() { MyObj stack; return Identity(stack); // expected-warning {{address of stack memory is returned later}} @@ -2419,15 +2505,13 @@ std::string_view return_dangling_view_through_owner() { return sv; // expected-note {{returned here}} } -// FIXME: False negative. Move assignment of unique_ptr is not defaulted, -// so origins from `local` don't propagate to `ups`. void owner_outlives_lifetimebound_source() { std::unique_ptr<S> ups; { std::string local; - ups = getUniqueS(local); - } - (void)ups; // Should warn. + ups = getUniqueS(local); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)ups; // expected-note {{later used here}} } } // namespace track_origins_for_lifetimebound_record_type `````````` </details> https://github.com/llvm/llvm-project/pull/191632 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
