Author: Kashika Akhouri Date: 2025-12-17T23:51:10+05:30 New Revision: 658df648d993630d9d2d1dea9402b6cd722bcbb8
URL: https://github.com/llvm/llvm-project/commit/658df648d993630d9d2d1dea9402b6cd722bcbb8 DIFF: https://github.com/llvm/llvm-project/commit/658df648d993630d9d2d1dea9402b6cd722bcbb8.diff LOG: [LifetimeSafety] Cross-TU Vs Intra-TU Annotation Suggestions (#171972) Differentiate between cross-TU and intra-TU annotation suggestions. This PR refines the lifetime annotation suggestion feature by introducing a distinction between cross-TU and intra-TU annotation suggestions. The primary purpose of this change is to differentiate between suggestions which can be replaced by inference and other cross-tu suggestions where inference is not sufficient to replace the need of explicit annotations. We have introduced two new warning groups under Lifetime Safety Suggestions: - `-Wexperimental-lifetime-safety-cross-tu-suggestions`: These are for functions whose declarations are in header, definition in source file. These are higher priority as it affects callers in other translation units. Inference cannot replace these annotations across TU boundaries (even in its most powerful form). Annotation suggestion is on the header declaration. - `-Wexperimental-lifetime-safety-intra-tu-suggestions`: For suggestions on functions which are in the same TU. These can be considered lower-priority suggestions as inference can potentially handle these within the same TU. Example: ```cpp // Header file r.h #include <iostream> #include <string> std::string_view public_func(std::string_view a); inline std::string_view inline_header(std::string_view a) { return a; } ``` ```cpp // Source (cpp file) #include <iostream> #include <string> #include "r.h" std::string_view public_func(std::string_view a) { return a; } namespace { std::string_view private_func(std::string_view a) { return a; } } ``` The warnings generated are: ``` ./r.h:6:39: warning: parameter in intra-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-intra-tu-suggestions] 6 | inline std::string_view inline_header(std::string_view a) { | ^~~~~~~~~~~~~~~~~~ | [[clang::lifetimebound]] ./r.h:7:12: note: param returned here 7 | return a; ./r.h:4:30: warning: parameter in cross-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-cross-tu-suggestions] 4 | std::string_view public_func(std::string_view a); | ^~~~~~~~~~~~~~~~~~ | [[clang::lifetimebound]] r.cpp:6:10: note: param returned here 6 | return a; | ^ r.cpp:10:33: warning: parameter in intra-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-intra-tu-suggestions] 10 | std::string_view private_func(std::string_view a) { | ^~~~~~~~~~~~~~~~~~ | [[clang::lifetimebound]] r.cpp:11:10: note: param returned here 11 | return a; | ^ ``` Added: Modified: clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp clang/test/Sema/warn-lifetime-safety-suggestions.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 2b3a220edc073..53caaa40650e2 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -34,6 +34,12 @@ enum class Confidence : uint8_t { Definite // Reported as a definite error (-Wlifetime-safety-permissive) }; +/// Enum to track functions visible across or within TU. +enum class SuggestionScope { + CrossTU, // For suggestions on declarations visible across Translation Units. + IntraTU // For suggestions on definitions local to a Translation Unit. +}; + class LifetimeSafetyReporter { public: LifetimeSafetyReporter() = default; @@ -49,7 +55,8 @@ class LifetimeSafetyReporter { Confidence Confidence) {} // Suggests lifetime bound annotations for function paramters - virtual void suggestAnnotation(const ParmVarDecl *PVD, + virtual void suggestAnnotation(SuggestionScope Scope, + const ParmVarDecl *ParmToAnnotate, const Expr *EscapeExpr) {} }; diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e1dba0195f470..7538066a7d644 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -541,8 +541,18 @@ def LifetimeSafety : DiagGroup<"experimental-lifetime-safety", Experimental warnings to detect use-after-free and related temporal safety bugs based on lifetime safety analysis. }]; } +def LifetimeSafetyCrossTUSuggestions + : DiagGroup<"experimental-lifetime-safety-cross-tu-suggestions">; +def LifetimeSafetyIntraTUSuggestions + : DiagGroup<"experimental-lifetime-safety-intra-tu-suggestions">; def LifetimeSafetySuggestions - : DiagGroup<"experimental-lifetime-safety-suggestions">; + : DiagGroup<"experimental-lifetime-safety-suggestions", + [LifetimeSafetyCrossTUSuggestions, + LifetimeSafetyIntraTUSuggestions]> { + code Documentation = [{ + Lifetime annotation suggestions for function parameters that should be marked [[clang::lifetimebound]] based on lifetime analysis. + }]; +} def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a8fd17ecd2bdd..49eee0c2fa617 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10815,9 +10815,16 @@ def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; def note_lifetime_safety_returned_here : Note<"returned here">; -def warn_lifetime_safety_suggest_lifetimebound - : Warning<"param should be marked [[clang::lifetimebound]]">, - InGroup<LifetimeSafetySuggestions>, +def warn_lifetime_safety_intra_tu_suggestion + : Warning<"parameter in intra-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyIntraTUSuggestions>, + DefaultIgnore; + +def warn_lifetime_safety_cross_tu_suggestion + : Warning<"parameter in cross-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyCrossTUSuggestions>, DefaultIgnore; def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 99071d6b46c1e..ff2650be594f5 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -20,6 +20,7 @@ #include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" @@ -160,11 +161,36 @@ class LifetimeChecker { } } + /// Returns the declaration of a function that is visible across translation + /// units, if such a declaration exists and is diff erent from the definition. + static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, + SourceManager &SM) { + const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext()); + if (!FD) + return nullptr; + if (!FD->isExternallyVisible()) + return nullptr; + const FileID DefinitionFile = SM.getFileID(FD->getLocation()); + for (const FunctionDecl *Redecl : FD->redecls()) + if (SM.getFileID(Redecl->getLocation()) != DefinitionFile) + return Redecl; + + return nullptr; + } + void suggestAnnotations() { if (!Reporter) return; - for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) - Reporter->suggestAnnotation(PVD, EscapeExpr); + SourceManager &SM = AST.getSourceManager(); + for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) { + if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) + Reporter->suggestAnnotation( + SuggestionScope::CrossTU, + CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), + EscapeExpr); + else + Reporter->suggestAnnotation(SuggestionScope::IntraTU, PVD, EscapeExpr); + } } void inferAnnotations() { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 2dbbfc5e90a17..9dcae980def25 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2883,14 +2883,27 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << EscapeExpr->getEndLoc(); } - void suggestAnnotation(const ParmVarDecl *PVD, + void suggestAnnotation(SuggestionScope Scope, + const ParmVarDecl *ParmToAnnotate, const Expr *EscapeExpr) override { + unsigned DiagID; + switch (Scope) { + case SuggestionScope::CrossTU: + DiagID = diag::warn_lifetime_safety_cross_tu_suggestion; + break; + case SuggestionScope::IntraTU: + DiagID = diag::warn_lifetime_safety_intra_tu_suggestion; + break; + } + SourceLocation InsertionPoint = Lexer::getLocForEndOfToken( - PVD->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts()); - S.Diag(PVD->getBeginLoc(), diag::warn_lifetime_safety_suggest_lifetimebound) - << PVD->getSourceRange() + ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts()); + + S.Diag(ParmToAnnotate->getBeginLoc(), DiagID) + << ParmToAnnotate->getSourceRange() << FixItHint::CreateInsertion(InsertionPoint, " [[clang::lifetimebound]]"); + S.Diag(EscapeExpr->getBeginLoc(), diag::note_lifetime_safety_suggestion_returned_here) << EscapeExpr->getSourceRange(); diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index fd79c497ea5d5..8a328dfbc8d9e 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -1,4 +1,12 @@ -// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -Wno-dangling -verify %s +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -Wno-dangling -I%t -verify %t/test_source.cpp + +View definition_before_header(View a); + +//--- test_header.h +#ifndef TEST_HEADER_H +#define TEST_HEADER_H struct View; @@ -16,14 +24,43 @@ struct [[gsl::Pointer()]] View { void use() const; }; -View return_view_directly (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View definition_before_header(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +View return_view_directly(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +View conditional_return_view( + View a, // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + View b, // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + bool c); + +int* return_pointer_directly(int* a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +MyObj* return_pointer_object(MyObj* a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +inline View inline_header_return_view(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} + return a; // expected-note {{param returned here}} +} + +View redeclared_in_header(View a); +inline View redeclared_in_header(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} + return a; // expected-note {{param returned here}} +} + +#endif // TEST_HEADER_H + +//--- test_source.cpp + +#include "test_header.h" + +View definition_before_header(View a) { + return a; // expected-note {{param returned here}} +} + +View return_view_directly(View a) { return a; // expected-note {{param returned here}} } -View conditional_return_view( - View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - bool c) { +View conditional_return_view(View a, View b, bool c) { View res; if (c) res = a; @@ -32,8 +69,16 @@ View conditional_return_view( return res; // expected-note 2 {{param returned here}} } -MyObj& return_reference(MyObj& a, // expected-warning {{param should be marked [[clang::lifetimebound]]}} - MyObj& b, // expected-warning {{param should be marked [[clang::lifetimebound]]}} +int* return_pointer_directly(int* a) { + return a; // expected-note {{param returned here}} +} + +MyObj* return_pointer_object(MyObj* a) { + return a; // expected-note {{param returned here}} +} + +MyObj& return_reference(MyObj& a, // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} + MyObj& b, // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} bool c) { if(c) { return a; // expected-note {{param returned here}} @@ -41,11 +86,11 @@ MyObj& return_reference(MyObj& a, // expected-warning {{param should be marked [ return b; // expected-note {{param returned here}} } -const MyObj& return_reference_const(const MyObj& a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}} +const MyObj& return_reference_const(const MyObj& a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} return a; // expected-note {{param returned here}} } -MyObj* return_ptr_to_ref(MyObj& a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}} +MyObj* return_ptr_to_ref(MyObj& a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} return &a; // expected-note {{param returned here}} } @@ -54,7 +99,7 @@ MyObj& return_ref_to_ptr(MyObj* a) { return *a; } -View return_view_from_reference(MyObj& p) { // expected-warning {{param should be marked [[clang::lifetimebound]]}} +View return_view_from_reference(MyObj& p) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} return p; // expected-note {{param returned here}} } @@ -66,37 +111,57 @@ struct Container { View return_struct_field(const Container& c) { return c.data; } -View return_struct_lifetimebound_getter(Container& c) { // expected-warning {{param should be marked [[clang::lifetimebound]]}} +View return_struct_lifetimebound_getter(Container& c) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} return c.getData().getView(); // expected-note {{param returned here}} } -View return_view_from_reference_lifetimebound_member(MyObj& p) { // expected-warning {{param should be marked [[clang::lifetimebound]]}} +View return_view_from_reference_lifetimebound_member(MyObj& p) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} return p.getView(); // expected-note {{param returned here}} } -int* return_pointer_directly (int* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - return a; // expected-note {{param returned here}} + +View return_cross_tu_func_View(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return return_view_directly(a); // expected-note {{param returned here}} } -MyObj* return_pointer_object (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. - return a; // expected-note {{param returned here}} +MyObj* return_cross_tu_func_obj(MyObj* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return return_pointer_object(a); // expected-note {{param returned here}} +} + +int* return_cross_tu_func_pointer(int* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return return_pointer_directly(a); // expected-note {{param returned here}} } -View only_one_paramter_annotated (View a [[clang::lifetimebound]], - View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +namespace { +View only_one_paramter_annotated(View a [[clang::lifetimebound]], + View b, // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. bool c) { if(c) return a; return b; // expected-note {{param returned here}} } -View reassigned_to_another_parameter ( +View reassigned_to_another_parameter( View a, - View b) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. + View b) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. a = b; return a; // expected-note {{param returned here}} } +View intra_tu_func_redecl(View a); +View intra_tu_func_redecl(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return a; // expected-note {{param returned here}} +} +} + +static View return_view_static(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return a; // expected-note {{param returned here}} +} + +//===----------------------------------------------------------------------===// +// FIXME Test Cases +//===----------------------------------------------------------------------===// + struct ReturnsSelf { const ReturnsSelf& get() const { return *this; @@ -127,11 +192,11 @@ void test_getView_on_temporary() { //===----------------------------------------------------------------------===// namespace correct_order_inference { -View return_view_by_func (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View return_view_by_func(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return return_view_directly(a); // expected-note {{param returned here}} } -MyObj* return_pointer_by_func (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +MyObj* return_pointer_by_func(MyObj* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return return_pointer_object(a); // expected-note {{param returned here}} } } // namespace correct_order_inference @@ -144,7 +209,7 @@ View return_view_caller(View a) { return return_view_callee(a); } -View return_view_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View return_view_callee(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } } // namespace incorrect_order_inference_view @@ -157,17 +222,17 @@ MyObj* return_object_caller(MyObj* a) { return return_object_callee(a); } -MyObj* return_object_callee(MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +MyObj* return_object_callee(MyObj* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } } // namespace incorrect_order_inference_object namespace simple_annotation_inference { -View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View inference_callee_return_identity(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } -View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View inference_caller_forwards_callee(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return inference_callee_return_identity(a); // expected-note {{param returned here}} } @@ -180,12 +245,12 @@ View inference_top_level_return_stack_view() { namespace inference_in_order_with_redecls { View inference_callee_return_identity(View a); -View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View inference_callee_return_identity(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } View inference_caller_forwards_callee(View a); -View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +View inference_caller_forwards_callee(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return inference_callee_return_identity(a); // expected-note {{param returned here}} } @@ -198,7 +263,7 @@ View inference_top_level_return_stack_view() { namespace inference_with_templates { template<typename T> -T* template_identity(T* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}. +T* template_identity(T* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return a; // expected-note {{param returned here}} } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
