https://github.com/hokein updated 
https://github.com/llvm/llvm-project/pull/122088

>From 82415b4085f3ab956926909d68b16afff26ceb51 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Tue, 17 Dec 2024 14:28:00 +0100
Subject: [PATCH 1/4] [clang] Refine the temporay object member access
 filtering for GSL pointer.

---
 clang/lib/Sema/CheckExprLifetime.cpp          | 42 +++++++++++++------
 clang/test/Sema/Inputs/lifetime-analysis.h    |  2 +-
 .../Sema/warn-lifetime-analysis-nocfg.cpp     | 28 +++++++++++++
 3 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp 
b/clang/lib/Sema/CheckExprLifetime.cpp
index 27e6b5b2cb3930..e144d23c6e7bb2 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -200,6 +200,7 @@ struct IndirectLocalPathEntry {
     LifetimeBoundCall,
     TemporaryCopy,
     LambdaCaptureInit,
+    MemberExpr,
     GslReferenceInit,
     GslPointerInit,
     GslPointerAssignment,
@@ -593,19 +594,6 @@ static void visitFunctionCallArguments(IndirectLocalPath 
&Path, Expr *Call,
     Path.pop_back();
   };
   auto VisitGSLPointerArg = [&](const FunctionDecl *Callee, Expr *Arg) {
-    // We are not interested in the temporary base objects of gsl Pointers:
-    //   Temp().ptr; // Here ptr might not dangle.
-    if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
-      return;
-    // Avoid false positives when the object is constructed from a conditional
-    // operator argument. A common case is:
-    //   // 'ptr' might not be owned by the Owner object.
-    //   std::string_view s = cond() ? Owner().ptr : sv;
-    if (const auto *Cond =
-            dyn_cast<AbstractConditionalOperator>(Arg->IgnoreImpCasts());
-        Cond && isPointerLikeType(Cond->getType()))
-      return;
-
     auto ReturnType = Callee->getReturnType();
 
     // Once we initialized a value with a non gsl-owner reference, it can no
@@ -726,6 +714,9 @@ static void 
visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
         Init = ILE->getInit(0);
     }
 
+    if (MemberExpr *ME = dyn_cast<MemberExpr>(Init->IgnoreImpCasts()))
+      Path.push_back(
+          {IndirectLocalPathEntry::MemberExpr, ME, ME->getMemberDecl()});
     // Step over any subobject adjustments; we may have a materialized
     // temporary inside them.
     Init = const_cast<Expr *>(Init->skipRValueSubobjectAdjustments());
@@ -1119,6 +1110,8 @@ shouldLifetimeExtendThroughPath(const IndirectLocalPath 
&Path) {
   for (auto Elem : Path) {
     if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
       return PathLifetimeKind::Extend;
+    if (Elem.Kind == IndirectLocalPathEntry::MemberExpr)
+      continue;
     if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
       return PathLifetimeKind::NoExtend;
   }
@@ -1138,6 +1131,7 @@ static SourceRange nextPathEntryRange(const 
IndirectLocalPath &Path, unsigned I,
     case IndirectLocalPathEntry::GslPointerInit:
     case IndirectLocalPathEntry::GslPointerAssignment:
     case IndirectLocalPathEntry::ParenAggInit:
+    case IndirectLocalPathEntry::MemberExpr:
       // These exist primarily to mark the path as not permitting or
       // supporting lifetime extension.
       break;
@@ -1167,6 +1161,7 @@ static bool pathOnlyHandlesGslPointer(const 
IndirectLocalPath &Path) {
     case IndirectLocalPathEntry::VarInit:
     case IndirectLocalPathEntry::AddressOf:
     case IndirectLocalPathEntry::LifetimeBoundCall:
+    case IndirectLocalPathEntry::MemberExpr:
       continue;
     case IndirectLocalPathEntry::GslPointerInit:
     case IndirectLocalPathEntry::GslReferenceInit:
@@ -1200,6 +1195,26 @@ static AnalysisResult analyzePathForGSLPointer(const 
IndirectLocalPath &Path,
   // At this point, Path represents a series of operations involving a
   // GSLPointer, either in the process of initialization or assignment.
 
+  // Process  temporary base objects for MemberExpr cases, e.g. Temp().field.
+  for (const auto &E : Path) {
+    if (E.Kind == IndirectLocalPathEntry::MemberExpr) {
+      // Avoid interfering  with the local base object.
+      if (pathContainsInit(Path))
+        return Abandon;
+
+      // We are not interested in the temporary base objects of gsl Pointers:
+      //   auto p1 = Temp().ptr; // Here p1 might not dangle.
+      // However, we want to diagnose for gsl owner fields:
+      //   auto p2 = Temp().owner; // Here p2 is dangling.
+      if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D);
+          FD && !FD->getType()->isReferenceType() &&
+          isRecordWithAttr<OwnerAttr>(FD->getType())) {
+        return Report;
+      }
+      return Abandon;
+    }
+  }
+
   // Note: A LifetimeBoundCall can appear interleaved in this sequence.
   // For example:
   //    const std::string& Ref(const std::string& a [[clang::lifetimebound]]);
@@ -1527,6 +1542,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const 
InitializedEntity *InitEntity,
 
       case IndirectLocalPathEntry::LifetimeBoundCall:
       case IndirectLocalPathEntry::TemporaryCopy:
+      case IndirectLocalPathEntry::MemberExpr:
       case IndirectLocalPathEntry::GslPointerInit:
       case IndirectLocalPathEntry::GslReferenceInit:
       case IndirectLocalPathEntry::GslPointerAssignment:
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h 
b/clang/test/Sema/Inputs/lifetime-analysis.h
index f888e6ab94bb6a..d318033ff0cc45 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -52,7 +52,7 @@ struct vector {
 
   void push_back(const T&);
   void push_back(T&&);
-  
+  const T& back() const;
   void insert(iterator, T&&);
 };
 
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp 
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 4c19367bb7f3dd..3b271af9dfa13f 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -806,3 +806,31 @@ std::string_view test2(int c, std::string_view sv) {
 }
 
 } // namespace GH120206
+
+namespace GH120543 {
+struct S {
+  std::string_view sv;
+  std::string s;
+};
+struct Q {
+  const S* get() const [[clang::lifetimebound]];
+};
+void test1() {
+  std::string_view k1 = S().sv; // OK
+  std::string_view k2 = S().s; // expected-warning {{object backing the 
pointer will}}
+  
+  std::string_view k3 = Q().get()->sv; // OK
+  std::string_view k4  = Q().get()->s; // expected-warning {{object backing 
the pointer will}}
+}
+
+struct Bar {};
+struct Foo {
+  std::vector<Bar> v;
+};
+Foo getFoo();
+void test2() {
+  const Foo& foo = getFoo();
+  const Bar& bar = foo.v.back(); // OK
+}
+
+} // namespace GH120543

>From 765d56630ca4cf4583bfb4c954f00ec3a4da4fb9 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Thu, 9 Jan 2025 09:03:22 +0100
Subject: [PATCH 2/4] Add a lifetimebound testcase

---
 clang/test/Sema/warn-lifetime-analysis-nocfg.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp 
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 3b271af9dfa13f..48f52b7fc2788c 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -815,12 +815,18 @@ struct S {
 struct Q {
   const S* get() const [[clang::lifetimebound]];
 };
+
+std::string_view foo(std::string_view sv [[clang::lifetimebound]]);
+
 void test1() {
   std::string_view k1 = S().sv; // OK
   std::string_view k2 = S().s; // expected-warning {{object backing the 
pointer will}}
   
   std::string_view k3 = Q().get()->sv; // OK
   std::string_view k4  = Q().get()->s; // expected-warning {{object backing 
the pointer will}}
+
+  std::string_view lb1 = foo(S().s); // expected-warning {{object backing the 
pointer will}}
+  std::string_view lb2 = foo(Q().get()->s); // expected-warning {{object 
backing the pointer will}}
 }
 
 struct Bar {};

>From 87fea691ea116907f6b93e1703f218bd9a5ada0b Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Mon, 13 Jan 2025 11:52:49 +0100
Subject: [PATCH 3/4] address review comment.

---
 clang/lib/Sema/CheckExprLifetime.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp 
b/clang/lib/Sema/CheckExprLifetime.cpp
index e144d23c6e7bb2..b3ee993c17de8c 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1108,12 +1108,12 @@ enum PathLifetimeKind {
 static PathLifetimeKind
 shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
   for (auto Elem : Path) {
-    if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
-      return PathLifetimeKind::Extend;
-    if (Elem.Kind == IndirectLocalPathEntry::MemberExpr)
+    if (Elem.Kind == IndirectLocalPathEntry::MemberExpr ||
+        Elem.Kind == IndirectLocalPathEntry::LambdaCaptureInit)
       continue;
-    if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
-      return PathLifetimeKind::NoExtend;
+    return Elem.Kind == IndirectLocalPathEntry::DefaultInit
+               ? PathLifetimeKind::Extend
+               : PathLifetimeKind::NoExtend;
   }
   return PathLifetimeKind::Extend;
 }

>From 3276f4ef1fa3ea9c5b27f4f7eac055c594737d74 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Mon, 20 Jan 2025 15:58:42 +0100
Subject: [PATCH 4/4] Fix a false postive in member initializer.

---
 clang/lib/Sema/CheckExprLifetime.cpp             |  8 +++++---
 clang/test/Sema/warn-lifetime-analysis-nocfg.cpp | 12 ++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp 
b/clang/lib/Sema/CheckExprLifetime.cpp
index b3ee993c17de8c..8963cad86dbcae 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1188,7 +1188,7 @@ enum AnalysisResult {
 // Analyze cases where a GSLPointer is initialized or assigned from a
 // temporary owner object.
 static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
-                                               Local L) {
+                                               Local L, LifetimeKind LK) {
   if (!pathOnlyHandlesGslPointer(Path))
     return NotGSLPointer;
 
@@ -1208,7 +1208,8 @@ static AnalysisResult analyzePathForGSLPointer(const 
IndirectLocalPath &Path,
       //   auto p2 = Temp().owner; // Here p2 is dangling.
       if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D);
           FD && !FD->getType()->isReferenceType() &&
-          isRecordWithAttr<OwnerAttr>(FD->getType())) {
+          isRecordWithAttr<OwnerAttr>(FD->getType()) &&
+          LK != LK_MemInitializer) {
         return Report;
       }
       return Abandon;
@@ -1312,7 +1313,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const 
InitializedEntity *InitEntity,
     auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
 
     bool IsGslPtrValueFromGslTempOwner = true;
-    switch (analyzePathForGSLPointer(Path, L)) {
+    switch (analyzePathForGSLPointer(Path, L, LK)) {
     case Abandon:
       return false;
     case Skip:
@@ -1444,6 +1445,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const 
InitializedEntity *InitEntity,
         auto *DRE = dyn_cast<DeclRefExpr>(L);
         // Suppress false positives for code like the one below:
         //   Ctor(unique_ptr<T> up) : pointer(up.get()), owner(move(up)) {}
+        // FIXME: move this logic to analyzePathForGSLPointer.
         if (DRE && isRecordWithAttr<OwnerAttr>(DRE->getType()))
           return false;
 
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp 
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 48f52b7fc2788c..04bb1330ded4c2 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -839,4 +839,16 @@ void test2() {
   const Bar& bar = foo.v.back(); // OK
 }
 
+struct Foo2 {
+   std::unique_ptr<Bar> bar;
+};
+
+struct Test {
+  Test(Foo2 foo) : bar(foo.bar.get()), // OK
+      storage(std::move(foo.bar)) {};
+
+  Bar* bar;
+  std::unique_ptr<Bar> storage;
+};
+
 } // namespace GH120543

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to