llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: NeKon69 <details> <summary>Changes</summary> Adds `-Wlifetime-safety-meaningless-lifetimebound` to diagnose `[[clang::lifetimebound]]` annotations that have no effect because the parameter type cannot carry a lifetime. This currently diagnoses scalar parameters and `gsl::Owner` parameters, while still allowing references, pointers, `gsl::Pointer` values, and unannotated record values (because their contents are ambiguous). Closes #<!-- -->177184 --- Full diff: https://github.com/llvm/llvm-project/pull/201101.diff 6 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+2) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+11-3) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+18) - (modified) clang/lib/Sema/SemaLifetimeSafety.h (+12-1) - (added) clang/test/Sema/warn-lifetime-safety-meaningless-lifetimebound.cpp (+76) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 398cce1395854..d3d9a523c9b19 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -141,6 +141,8 @@ class LifetimeSafetySemaHelper { const ParmVarDecl *PVDDef, const ParmVarDecl *PVDDecl) {} + virtual void reportMeaninglessLifetimebound(const ParmVarDecl *PVD) {} + // Suggests lifetime bound annotations for implicit this. virtual void suggestLifetimeboundToImplicitThis(WarningScope Scope, const CXXMethodDecl *MD, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 8031f99419bdc..17374d4aadae3 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -631,6 +631,13 @@ This warning may produce false-positives diagnostics when it cannot fully model }]; } +def LifetimeSafetyMeaninglessLifetimebound + : DiagGroup<"lifetime-safety-meaningless-lifetimebound"> { + code Documentation = [{ +Detects uses of [[clang::lifetimebound]] that have no effect because the annotated type cannot carry a lifetime. + }]; +} + def LifetimeSafetyCrossTUMisplacedLifetimebound : DiagGroup<"lifetime-safety-cross-tu-misplaced-lifetimebound">; def LifetimeSafetyIntraTUMisplacedLifetimebound @@ -686,9 +693,10 @@ Detects misuse of [[clang::noescape]] annotation where the parameter escapes (fo } def LifetimeSafetyValidations : DiagGroup<"lifetime-safety-validations", - [LifetimeSafetyNoescape, - LifetimeSafetyLifetimeboundViolation, - LifetimeSafetyMisplacedLifetimebound]> { + [LifetimeSafetyNoescape, + LifetimeSafetyLifetimeboundViolation, + LifetimeSafetyMeaninglessLifetimebound, + LifetimeSafetyMisplacedLifetimebound]> { code Documentation = [{ Verify function implementations adhere to the annotated lifetime contracts through lifetime safety like verifying [[clang::noescape]] and [[clang::lifetimebound]]. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 077aace321264..c6511a84d82bc 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11034,6 +11034,11 @@ def warn_lifetime_safety_cross_tu_misplaced_lifetimebound InGroup<LifetimeSafetyCrossTUMisplacedLifetimebound>, DefaultIgnore; +def warn_lifetime_safety_meaningless_lifetimebound + : Warning<"'lifetimebound' attribute has no effect on parameter of type %0">, + InGroup<LifetimeSafetyMeaninglessLifetimebound>, + DefaultIgnore; + def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_invalidated_here : Note<"invalidated here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index d6d4ec6b5617e..a35069f49f460 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -104,6 +104,7 @@ class LifetimeChecker { reportNoescapeViolations(); reportLifetimeboundViolations(); reportMisplacedLifetimebound(); + reportMeaninglessLifetimebound(); // Annotation inference is currently guarded by a frontend flag. In the // future, this might be replaced by a design that differentiates between // explicit and inferred findings with separate warning groups. @@ -467,6 +468,23 @@ class LifetimeChecker { } } + static bool canBeLifetimebound(QualType QT) { + if (QT->isReferenceType() || QT->isPointerType()) + return true; + if (isGslOwnerType(QT) || QT->isScalarType()) + return false; + return true; + } + + void reportMeaninglessLifetimebound() { + for (const auto &PVD : cast<FunctionDecl>(FD)->parameters()) { + if (!PVD->hasAttr<LifetimeBoundAttr>()) + continue; + if (!canBeLifetimebound(PVD->getType())) + SemaHelper->reportMeaninglessLifetimebound(PVD); + } + } + void inferAnnotations() { for (auto [Target, EscapeTarget] : AnnotationWarningsMap) { if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) { diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 6da4953dea56d..6b070b07d36e2 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -20,6 +20,7 @@ #include "clang/Basic/DiagnosticSema.h" #include "clang/Lex/Lexer.h" #include "clang/Sema/Sema.h" +#include <clang/AST/Attrs.inc> #include <string> namespace clang::lifetimes { @@ -48,7 +49,8 @@ inline bool IsLifetimeSafetyEnabled(Sema &S, const Decl *D) { diag::warn_lifetime_safety_cross_tu_param_suggestion, diag::warn_lifetime_safety_intra_tu_param_suggestion, diag::warn_lifetime_safety_cross_tu_this_suggestion, - diag::warn_lifetime_safety_intra_tu_this_suggestion}; + diag::warn_lifetime_safety_intra_tu_this_suggestion, + diag::warn_lifetime_safety_meaningless_lifetimebound}; for (unsigned DiagID : DiagIDs) if (!Diags.isIgnored(DiagID, D->getBeginLoc())) return true; @@ -343,6 +345,15 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << Attr->getRange(); } + void reportMeaninglessLifetimebound(const ParmVarDecl *PVD) override { + assert(PVD->hasAttr<LifetimeBoundAttr>() && + "Expected parameter to have lifetimebound attribute"); + const auto *Attr = PVD->getAttr<LifetimeBoundAttr>(); + S.Diag(Attr->getLocation(), + diag::warn_lifetime_safety_meaningless_lifetimebound) + << PVD->getType() << Attr->getRange(); + } + void suggestLifetimeboundToImplicitThis(WarningScope Scope, const CXXMethodDecl *MD, const Expr *EscapeExpr) override { diff --git a/clang/test/Sema/warn-lifetime-safety-meaningless-lifetimebound.cpp b/clang/test/Sema/warn-lifetime-safety-meaningless-lifetimebound.cpp new file mode 100644 index 0000000000000..39345d484aaec --- /dev/null +++ b/clang/test/Sema/warn-lifetime-safety-meaningless-lifetimebound.cpp @@ -0,0 +1,76 @@ +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety-meaningless-lifetimebound -Wno-dangling -verify %s + +struct [[gsl::Owner]] Owner {}; + +struct [[gsl::Pointer()]] View { + View(); + View(const Owner &o [[clang::lifetimebound]]); +}; + +struct Plain {}; + +enum Enum { Enumerator }; + +struct Mixed { + Mixed(const int &i [[clang::lifetimebound]]); +}; + +using IntAlias = int; + +Owner *owner_value(Owner o [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'Owner'}} + return {}; +} + +Owner *const_owner_value(const Owner o [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'const Owner'}} + return {}; +} + +Owner *owner_ref(Owner &o [[clang::lifetimebound]]) { + return &o; +} + +const Owner *const_owner_ref(const Owner &o [[clang::lifetimebound]]) { + return &o; +} + +Owner *owner_ptr(Owner *o [[clang::lifetimebound]]) { + return o; +} + +int *scalar_ptr(int *p [[clang::lifetimebound]]) { + return p; +} + +int *scalar_value(int i [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'int'}} + return {}; +} + +int *scalar_alias_value(IntAlias i [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'IntAlias' (aka 'int')}} + return {}; +} + +int *enum_value(Enum e [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'Enum'}} + return {}; +} + +View view_value(View v [[clang::lifetimebound]]) { + return v; +} + +Plain *plain_value(Plain p [[clang::lifetimebound]]) { + return {}; +} + +Mixed *mixed_value(Mixed m [[clang::lifetimebound]]) { + return {}; +} + +template <class T> +Owner *template_value(T t [[clang::lifetimebound]]) { // expected-warning {{'lifetimebound' attribute has no effect on parameter of type 'Owner'}} + return {}; +} + +void instantiate_template() { + Owner o; + (void)template_value(o); // expected-note {{in instantiation of function template specialization 'template_value<Owner>' requested here}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/201101 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
