llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

Author: NeKon69

<details>
<summary>Changes</summary>

This updates `[[clang::lifetimebound]]` verification to also diagnose implicit 
`this` parameters.

It also reports lifetimebound verification diagnostics at the attribute 
location, so declarations with the attribute now point at the declaration 
rather than only at the function definition.

---
Full diff: https://github.com/llvm/llvm-project/pull/196824.diff


5 Files Affected:

- (modified) 
clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+7-1) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2-2) 
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+9-4) 
- (modified) clang/lib/Sema/SemaLifetimeSafety.h (+18-4) 
- (modified) clang/test/Sema/warn-lifetime-safety-lifetimebound.cpp (+47-3) 


``````````diff
diff --git 
a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h 
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 7ccf30ba14987..b4f918a4cb3e7 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -110,7 +110,13 @@ class LifetimeSafetySemaHelper {
 
   // Reports misuse of [[clang::lifetimebound]] when parameter doesn't escape
   // through return.
-  virtual void reportLifetimeboundViolation(const ParmVarDecl *VD) {}
+  virtual void
+  reportLifetimeboundViolation(const ParmVarDecl *ParmWithLifetimebound) {}
+
+  // Reports misuse of [[clang::lifetimebound]] when implicit this parameter
+  // doesn't escape through return.
+  virtual void
+  reportLifetimeboundViolation(const CXXMethodDecl *MDWithLifetimebound) {}
 
   // Suggests lifetime bound annotations for implicit this.
   virtual void suggestLifetimeboundToImplicitThis(SuggestionScope Scope,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 879812f3de0d3..4374ca7235d03 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11007,8 +11007,8 @@ def warn_lifetime_safety_dangling_global_moved
       InGroup<LifetimeSafetyDanglingGlobalMoved>,
       DefaultIgnore;
 
-def warn_lifetime_safety_param_lifetimebound_violation
-    : Warning<"could not verify that the return value can be lifetime bound to 
%select{an unnamed parameter|'%1'}0">,
+def warn_lifetime_safety_lifetimebound_violation
+    : Warning<"could not verify that the return value can be lifetime bound to 
%select{an unnamed parameter|'%1'|an implicit this parameter}0">,
       InGroup<LifetimeSafetyLifetimeboundViolation>,
       DefaultIgnore;
 
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp 
b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index fc77ed3097602..f60448b23e6e8 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -60,7 +60,7 @@ class LifetimeChecker {
   llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap;
   llvm::DenseMap<AnnotationTarget, EscapingTarget> AnnotationWarningsMap;
   llvm::DenseMap<const ParmVarDecl *, EscapingTarget> NoescapeWarningsMap;
-  llvm::DenseSet<const ParmVarDecl *> VerifiedLiftimeboundEscapes;
+  llvm::DenseSet<const Decl *> VerifiedLiftimeboundEscapes;
   const LoanPropagationAnalysis &LoanPropagation;
   const MovedLoansAnalysis &MovedLoans;
   const LiveOriginsAnalysis &LiveOrigins;
@@ -147,9 +147,10 @@ class LifetimeChecker {
       // field!
     };
     auto CheckImplicitThis = [&](const CXXMethodDecl *MD) {
-      if (!implicitObjectParamIsLifetimeBound(MD))
-        if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF))
-          AnnotationWarningsMap.try_emplace(MD, ReturnEsc->getReturnExpr());
+      if (implicitObjectParamIsLifetimeBound(MD))
+        VerifiedLiftimeboundEscapes.insert(MD);
+      else if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF))
+        AnnotationWarningsMap.try_emplace(MD, ReturnEsc->getReturnExpr());
     };
     auto MovedAtEscape = MovedLoans.getMovedLoans(OEF);
     for (LoanID LID : EscapedLoans) {
@@ -366,6 +367,10 @@ class LifetimeChecker {
   void reportLifetimeboundViolations() {
     if (!isa<FunctionDecl>(FD))
       return;
+    if (const auto *MD = dyn_cast<CXXMethodDecl>(FD);
+        MD && implicitObjectParamIsLifetimeBound(MD) &&
+        !VerifiedLiftimeboundEscapes.contains(MD))
+      SemaHelper->reportLifetimeboundViolation(MD);
     for (const ParmVarDecl *PVD : cast<FunctionDecl>(FD)->parameters()) {
       if (!PVD->hasAttr<LifetimeBoundAttr>())
         continue;
diff --git a/clang/lib/Sema/SemaLifetimeSafety.h 
b/clang/lib/Sema/SemaLifetimeSafety.h
index 5b1cf41445399..91b1b96d40fb3 100644
--- a/clang/lib/Sema/SemaLifetimeSafety.h
+++ b/clang/lib/Sema/SemaLifetimeSafety.h
@@ -36,7 +36,7 @@ inline bool IsLifetimeSafetyDiagnosticEnabled(Sema &S, const 
Decl *D) {
       diag::warn_lifetime_safety_dangling_global,
       diag::warn_lifetime_safety_dangling_global_moved,
       diag::warn_lifetime_safety_noescape_escapes,
-      diag::warn_lifetime_safety_param_lifetimebound_violation,
+      diag::warn_lifetime_safety_lifetimebound_violation,
   };
   for (unsigned DiagID : DiagIDs)
     if (!Diags.isIgnored(DiagID, D->getBeginLoc()))
@@ -180,11 +180,25 @@ class LifetimeSafetySemaHelperImpl : public 
LifetimeSafetySemaHelper {
 
   void reportLifetimeboundViolation(
       const ParmVarDecl *ParmWithLifetimebound) override {
+    const auto *Attr = ParmWithLifetimebound->getAttr<LifetimeBoundAttr>();
     StringRef ParamName = ParmWithLifetimebound->getName();
     bool HasName = ParamName.size() > 0;
-    S.Diag(ParmWithLifetimebound->getLocation(),
-           diag::warn_lifetime_safety_param_lifetimebound_violation)
-        << HasName << ParamName << ParmWithLifetimebound->getSourceRange();
+    S.Diag(Attr->getLocation(),
+           diag::warn_lifetime_safety_lifetimebound_violation)
+        << HasName << ParamName << Attr->getRange();
+  }
+
+  void reportLifetimeboundViolation(
+      const CXXMethodDecl *MDWithLifetimebound) override {
+    const Stmt *Body = MDWithLifetimebound->getBody();
+    assert(Body && "Expected a body");
+    // FIXME: When #196549 lands, we can extract the attribute location and 
warn
+    // on it, for now warn on everything before the body.
+    S.Diag(MDWithLifetimebound->getLocation(),
+           diag::warn_lifetime_safety_lifetimebound_violation)
+        << 2 << "this"
+        << CharSourceRange::getCharRange(MDWithLifetimebound->getBeginLoc(),
+                                         Body->getBeginLoc());
   }
 
   void suggestLifetimeboundToImplicitThis(SuggestionScope Scope,
diff --git a/clang/test/Sema/warn-lifetime-safety-lifetimebound.cpp 
b/clang/test/Sema/warn-lifetime-safety-lifetimebound.cpp
index 941a3bb8ce1e3..04636ba70fc90 100644
--- a/clang/test/Sema/warn-lifetime-safety-lifetimebound.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-lifetimebound.cpp
@@ -81,9 +81,53 @@ View unnamed_lifetimebound_param(
   return View();
 }
 
-// FIXME: Should warn on declaration, not definiton
-View annotated_decl_but_not_def_not_returned(const MyObj &obj 
[[clang::lifetimebound]]);
+View annotated_decl_but_not_def_not_returned(const MyObj &obj 
[[clang::lifetimebound]]); // expected-warning {{could not verify that the 
return value can be lifetime bound to 'obj'}}
 
-View annotated_decl_but_not_def_not_returned(const MyObj &obj) { // 
expected-warning {{could not verify that the return value can be lifetime bound 
to 'obj'}}
+View annotated_decl_but_not_def_not_returned(const MyObj &obj) {
   return not_lb(obj);
 }
+
+struct BadThisReturn {
+  MyObj data;
+
+  View get() const [[clang::lifetimebound]] { // expected-warning {{could not 
verify that the return value can be lifetime bound to an implicit this 
parameter}}
+    return not_lb(data);
+  }
+};
+
+struct GoodThisReturn {
+  MyObj data;
+
+  View get() const [[clang::lifetimebound]] {
+    return data;
+  }
+};
+
+// FIXME: Wrong warning loc
+struct RedeclaredThis {
+  MyObj data;
+  View get() const [[clang::lifetimebound]];
+};
+
+View RedeclaredThis::get() const { // expected-warning {{could not verify that 
the return value can be lifetime bound to an implicit this parameter}}
+  return not_lb(data);
+}
+
+struct ThisAndParam {
+  MyObj data;
+
+  View get(const MyObj &obj [[clang::lifetimebound]]) const 
[[clang::lifetimebound]] { // expected-warning {{could not verify that the 
return value can be lifetime bound to an implicit this parameter}}
+    return lb(obj);
+  }
+};
+
+struct ThisAndMixedParams {
+  MyObj data;
+
+  View get( // expected-warning {{could not verify that the return value can 
be lifetime bound to an implicit this parameter}}
+      const MyObj &a [[clang::lifetimebound]],
+      const MyObj &b,
+      const MyObj &c [[clang::lifetimebound]]) const [[clang::lifetimebound]] 
{ // expected-warning {{could not verify that the return value can be lifetime 
bound to 'c'}}
+    return cond() ? lb(a) : not_lb(b);
+  }
+};

``````````

</details>


https://github.com/llvm/llvm-project/pull/196824
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to