llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: NeKon69 <details> <summary>Changes</summary> Most of the diagnostic's wording was taken from -Wreturn-stack-address with exceptions such as: - We do not special-case `[[clang::musttail]]` - We do not special-case `CompoundLiteralExpr` as it is mostly a C thing. This patch does not add any new tests, it only updates already existing test warnings to follow the new wording. --- Patch is 71.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/199432.diff 7 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+6-3) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+21-10) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+15-4) - (modified) clang/lib/Sema/SemaLifetimeSafety.h (+25-5) - (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+44-44) - (modified) clang/test/Sema/warn-lifetime-safety-suggestions.cpp (+13-11) - (modified) clang/test/Sema/warn-lifetime-safety.cpp (+106-106) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index ec9f83748ad8d..895d64713f2da 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -65,10 +65,13 @@ class LifetimeSafetySemaHelper { const Expr *MovedExpr, SourceLocation FreeLoc) {} - virtual void reportUseAfterReturn(const Expr *IssueExpr, + virtual void reportUseAfterReturn(const ValueDecl *VD, const Expr *IssueExpr, const Expr *ReturnExpr, - const Expr *MovedExpr, - SourceLocation ExpiryLoc) {} + const Expr *MovedExpr, bool IsReference) {} + + virtual void reportUseAfterReturn(const MaterializeTemporaryExpr *MTE, + const Expr *ReturnExpr, + const Expr *MovedExpr, bool IsReference) {} virtual void reportDanglingField(const Expr *IssueExpr, const FieldDecl *Field, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index dbe6cb2c3a41c..de0efde20a642 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10967,16 +10967,27 @@ def warn_lifetime_safety_use_after_free : Warning< "%select{allocated object|parameter}0 does not live long enough">, InGroup<LifetimeSafetyUseAfterFree>, DefaultIgnore; -def warn_lifetime_safety_return_stack_addr - : Warning<"address of stack memory is returned later">, - InGroup<LifetimeSafetyReturnStackAddr>, - DefaultIgnore; -def warn_lifetime_safety_return_stack_addr_moved - : Warning<"address of stack memory may be returned later. " - "This could be false positive as the storage may have been moved. " - "Consider moving first and then aliasing later to resolve the issue">, - InGroup<LifetimeSafetyReturnStackAddrMoved>, - DefaultIgnore; +def warn_lifetime_safety_return_stack_addr : Warning< + "%select{address of|reference to}0 stack memory associated with " + "%select{local variable|parameter}2 %1 returned">, + InGroup<LifetimeSafetyReturnStackAddr>, DefaultIgnore; + +def warn_lifetime_safety_return_temp_stack_addr : Warning< + "returning %select{address of|reference to}0 local temporary object">, + InGroup<LifetimeSafetyReturnStackAddr>, DefaultIgnore; + +def warn_lifetime_safety_return_temp_stack_addr_moved : Warning< + "returning %select{address of|reference to}0 local temporary object. " + "This could be a false positive as the storage may have been moved. " + "Consider moving first and then aliasing later to resolve the issue">, + InGroup<LifetimeSafetyReturnStackAddrMoved>, DefaultIgnore; + +def warn_lifetime_safety_return_stack_addr_moved : Warning< + "%select{address of|reference to}0 stack memory associated with " + "%select{local variable|parameter}2 %1 may be returned later. " + "This could be a false positive as the storage may have been moved. " + "Consider moving first and then aliasing later to resolve the issue">, + InGroup<LifetimeSafetyReturnStackAddrMoved>, DefaultIgnore; def warn_lifetime_safety_invalidation : Warning<"%select{object whose reference is captured|parameter}0 is later invalidated">, diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index d6a15139aa4ea..2a7b28afb0c9f 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -297,11 +297,22 @@ class LifetimeChecker { // FIXME: Diagnose invalidated return escapes separately. } else llvm_unreachable("Unhandled OriginEscapesFact type"); - } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) + } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) { // Return stack address. - SemaHelper->reportUseAfterReturn( - IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExpiryLoc); - else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) + const AccessPath &Path = L->getAccessPath(); + bool IsReference = + cast<FunctionDecl>(FD)->getReturnType()->isReferenceType(); + if (const auto *VD = Path.getAsValueDecl()) + SemaHelper->reportUseAfterReturn(VD, IssueExpr, + RetEscape->getReturnExpr(), + MovedExpr, IsReference); + else if (const auto *MTE = Path.getAsMaterializeTemporaryExpr()) + SemaHelper->reportUseAfterReturn(MTE, RetEscape->getReturnExpr(), + MovedExpr, IsReference); + else + llvm_unreachable( + "unexpected loan kind for return stack address warning"); + } else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) // Dangling field. SemaHelper->reportDanglingField( IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index af5202c33fed0..6bd78f79da802 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -74,12 +74,16 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << UseExpr->getSourceRange(); } - void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, + void reportUseAfterReturn(const ValueDecl *VD, const Expr *IssueExpr, + const Expr *ReturnExpr, const Expr *MovedExpr, - SourceLocation ExpiryLoc) override { - S.Diag(IssueExpr->getExprLoc(), - MovedExpr ? diag::warn_lifetime_safety_return_stack_addr_moved - : diag::warn_lifetime_safety_return_stack_addr) + bool IsReference) override { + unsigned DiagID = MovedExpr + ? diag::warn_lifetime_safety_return_stack_addr_moved + : diag::warn_lifetime_safety_return_stack_addr; + + S.Diag(IssueExpr->getExprLoc(), DiagID) + << IsReference << VD << isa<ParmVarDecl>(VD) << IssueExpr->getSourceRange(); if (MovedExpr) S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) @@ -88,6 +92,22 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << ReturnExpr->getSourceRange(); } + void reportUseAfterReturn(const MaterializeTemporaryExpr *MTE, + const Expr *ReturnExpr, + const Expr *MovedExpr, + bool IsReference) override { + unsigned DiagID = + MovedExpr ? diag::warn_lifetime_safety_return_temp_stack_addr_moved + : diag::warn_lifetime_safety_return_temp_stack_addr; + + S.Diag(MTE->getExprLoc(), DiagID) << IsReference << MTE->getSourceRange(); + if (MovedExpr) + S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) + << MovedExpr->getSourceRange(); + S.Diag(ReturnExpr->getExprLoc(), diag::note_lifetime_safety_returned_here) + << ReturnExpr->getSourceRange(); + } + void reportDanglingField(const Expr *IssueExpr, const FieldDecl *DanglingField, const Expr *MovedExpr, diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 00d45caae3b09..73a16f8184368 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -98,7 +98,7 @@ struct DanglingGslPtrField { MyIntPointer danglingGslPtrFromLocal() { int j; // Detected only by CFG analysis. - return &j; // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + return &j; // cfg-warning {{address of stack memory associated with local variable 'j' returned}} cfg-note {{returned here}} } MyIntPointer returningLocalPointer() { @@ -109,30 +109,30 @@ MyIntPointer returningLocalPointer() { MyIntPointer daglingGslPtrFromLocalOwner() { MyIntOwner localOwner; return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 'localOwner' returned}} cfg-note {{returned here}} } MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() { MyLongOwnerWithConversion localOwner; return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 'localOwner' returned}} cfg-note {{returned here}} } MyIntPointer danglingGslPtrFromTemporary() { return MyIntOwner{}; // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } MyIntOwner makeTempOwner(); MyIntPointer danglingGslPtrFromTemporary2() { return makeTempOwner(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() { return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } int *noFalsePositive(MyIntOwner &o) { @@ -180,7 +180,7 @@ struct LifetimeBoundCtor { }; auto lifetimebound_make_unique_single_param() { - return std::make_unique<LifetimeBoundCtor>(MyIntOwner{}); // tu-warning {{address of stack memory is returned later}} tu-note {{returned here}} + return std::make_unique<LifetimeBoundCtor>(MyIntOwner{}); // tu-warning {{returning address of local temporary object}} tu-note {{returned here}} } @@ -199,17 +199,17 @@ void modelIterators() { std::vector<int>::iterator modelIteratorReturn() { return std::vector<int>().begin(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } const int *modelFreeFunctions() { return std::data(std::vector<int>()); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } int &modelAnyCast() { return std::any_cast<int&>(std::any{}); // expected-warning {{returning reference to local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning reference to local temporary object}} cfg-note {{returned here}} } int modelAnyCast2() { @@ -223,19 +223,19 @@ int modelAnyCast3() { const char *danglingRawPtrFromLocal() { std::basic_string<char> s; return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 's' returned}} cfg-note {{returned here}} } int &danglingRawPtrFromLocal2() { std::optional<int> o; return o.value(); // expected-warning {{reference to stack memory associated with local variable 'o' returned}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{reference to stack memory associated with local variable 'o' returned}} cfg-note {{returned here}} } int &danglingRawPtrFromLocal3() { std::optional<int> o; return *o; // expected-warning {{reference to stack memory associated with local variable 'o' returned}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{reference to stack memory associated with local variable 'o' returned}} cfg-note {{returned here}} } // GH100384 @@ -254,14 +254,14 @@ std::string_view containerWithAnnotatedElements() { std::vector<std::string> local; return local.at(0); // expected-warning {{address of stack memory associated with local variable}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 'local' returned}} cfg-note {{returned here}} } std::string_view localUniquePtr(int i) { std::unique_ptr<std::string> c1; if (i) return *c1; // expected-warning {{address of stack memory associated with local variable}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 'c1' returned}} cfg-note {{returned here}} std::unique_ptr<std::string_view> c2; return *c2; // expect no-warning. } @@ -270,38 +270,38 @@ std::string_view localOptional(int i) { std::optional<std::string> o; if (i) return o.value(); // expected-warning {{address of stack memory associated with local variable}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with local variable 'o' returned}} cfg-note {{returned here}} std::optional<std::string_view> abc; return abc.value(); // expect no warning } const char *danglingRawPtrFromTemp() { return std::basic_string<char>().c_str(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } std::unique_ptr<int> getUniquePtr(); int *danglingUniquePtrFromTemp() { return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } int *danglingUniquePtrFromTemp2() { return std::unique_ptr<int>().get(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } const int& danglingRefToOptionalFromTemp3() { return std::optional<int>().value(); // expected-warning {{returning reference to local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning reference to local temporary object}} cfg-note {{returned here}} } std::optional<std::string> getTempOptStr(); std::string_view danglingRefToOptionalFromTemp4() { return getTempOptStr().value(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } void danglingReferenceFromTempOwner() { @@ -346,14 +346,14 @@ int &usedToBeFalsePositive(std::vector<int> &v) { int &doNotFollowReferencesForLocalOwner() { // Warning caught by CFG analysis. std::unique_ptr<int> localOwner; - int &p = *localOwner // cfg-warning {{address of stack memory is returned later}} + int &p = *localOwner // cfg-warning {{reference to stack memory associated with local variable 'localOwner' returned}} .get(); return p; // cfg-note {{returned here}} } const char *trackThroughMultiplePointer() { return std::basic_string_view<char>(std::basic_string<char>()).begin(); // expected-warning {{returning address of local temporary object}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} } struct X { @@ -447,11 +447,11 @@ std::vector<std::string_view> GetTemporaryView(); std::string_view test_str_local() { std::vector<std::string> v; - return *std::find(v.begin(), // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + return *std::find(v.begin(), // cfg-warning {{address of stack memory associated with local variable 'v' returned}} cfg-note {{returned here}} v.end(), "42"); } std::string_view test_str_temporary() { - return *std::find(GetTemporaryString().begin(), // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + return *std::find(GetTemporaryString().begin(), // cfg-warning {{returning address of local temporary object}} cfg-note {{returned here}} GetTemporaryString().end(), "42"); } std::string_view test_view() { @@ -566,7 +566,7 @@ struct [[gsl::Pointer]] S { S test(std::vector<int> a) { return S(a); // expected-warning {{address of stack memory associated with}} \ - // cfg-warning {{address of stack memory is returned later}} cfg-note {{returned here}} + // cfg-warning {{address of stack memory associated with parameter 'a' returned}} cfg-note {{returned here}} } // FIXME: Detect this using the CFG-based lifetime analysis (global initialisation). @@ -586,10 +586,10 @@ struct FooView { FooView test3(int i, std::optional<Foo> a) { if (i) return *a; // expected-warning {{address of stack memory}} \ - // cfg-warning {{address of stack memory is returned later}} \ + // cfg-warning {{address of stack memory associated with parameter 'a' returned}} \ // cfg-note {{returned here}} return a.value(); // expected-warning {{address of stack memory}} \ - // cfg-warning {{address of stack memory is returned later}} \ + // cfg-warning {{address of stack memory associated with parameter 'a' returned}} \ // cfg-note {{returned here}} } } // namespace GH93386 @@ -649,7 +649,7 @@ st... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/199432 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
