[clang] Extend lifetimebound analysis to detect within-initializer assignments between pointer-like objects.

[hokein]

This patch extend the lifetimebound analysis to cover assignments between pointer-like objects (gsl::Pointer). Specifically, it tracks the RHS of assignment operators to determine if it points to a temporary object whose lifetime ends at the end of the full expression. If such a situation is detected, a diagnostic warning is emitted.

Note that this detection currently applies only to the assignment expression within the initializer:

/tmp/t.cpp:3:30: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
    3 |   std::string_view b = abc = std::string();
                                     ^~~~~~~~~~~~~

[llvmbot]

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

This patch extend the lifetimebound analysis to cover assignments between pointer-like objects (gsl::Pointer). Specifically, it tracks the RHS of assignment operators to determine if it points to a temporary object whose lifetime ends at the end of the full expression. If such a situation is detected, a diagnostic warning is emitted.

Note that this detection currently applies only to the assignment expression within the initializer:

/tmp/t.cpp:3:30: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
    3 |   std::string_view b = abc = std::string();
                                     ^~~~~~~~~~~~~

Part of #63310

---

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

2 Files Affected:

• (modified) clang/lib/Sema/CheckExprLifetime.cpp (+49-20)
• (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+1)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 2f9ef28da2c3e..f2fd3849b0448 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -320,6 +320,33 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
   return false;
 }
 
+// Returns true if the FD is a normal assignment operator.
+//
+// Assume that all assignment operators with a "normal" return type return
+// *this, that is, an lvalue reference that is the same type as the implicit
+// object parameter (or the LHS for a non-member operator$=).
+static bool isNormalAssignmentOperator(const FunctionDecl* FD) {
+  if (!FD)
+    return false;
+  OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
+  if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) {
+    QualType RetT = FD->getReturnType();
+    if (RetT->isLValueReferenceType()) {
+      ASTContext &Ctx = FD->getASTContext();
+      QualType LHST;
+      auto *MD = dyn_cast<CXXMethodDecl>(FD);
+      if (MD && MD->isCXXInstanceMember())
+        LHST = Ctx.getLValueReferenceType(MD->getFunctionObjectParameterType());
+      else
+        LHST = MD->getParamDecl(0)->getType();
+      if (Ctx.hasSameType(RetT, LHST))
+        return true;
+    }
+  }
+  return false;
+}
+
+
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
                                     LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
@@ -362,6 +389,26 @@ static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
         shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee)))
       VisitPointerArg(Callee, OCE->getArg(0),
                       !Callee->getReturnType()->isReferenceType());
+    // For assignment operators, if a pointer-type object (LHS) is assigned a
+    // pointer-type object (RHS), we further track the RHS expression to see
+    // whether it holds a temporary object that will be destroyed at the end of
+    // the full expression.
+    // This addresses the common scenario where a temporary owner-type object is
+    // assigned to a pointer-type object:
+    //
+    //   // On the RHS, we construct a temporary string_view from the string(),
+    //   // and invoke the operator=(const basic_string_view&) method.
+    //   my_string_view_var = std::string();
+    //
+    // FIXME: support assignment operator whose RHS parameter type is an owner
+    // type.
+    if (isNormalAssignmentOperator(Callee)) {
+      auto LHSType = OCE->getArg(0)->getType();
+      auto RHSType = OCE->getArg(1)->getType();
+      if (isRecordWithAttr<PointerAttr>(RHSType) &&
+          Callee->getASTContext().hasSameUnqualifiedType(LHSType, RHSType))
+        VisitPointerArg(Callee, OCE->getArg(1), true);
+    }
     return;
   } else if (auto *CE = dyn_cast<CallExpr>(Call)) {
     FunctionDecl *Callee = CE->getDirectCallee();
@@ -394,26 +441,8 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
       return true;
   }
 
-  // Assume that all assignment operators with a "normal" return type return
-  // *this, that is, an lvalue reference that is the same type as the implicit
-  // object parameter (or the LHS for a non-member operator$=).
-  OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
-  if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) {
-    QualType RetT = FD->getReturnType();
-    if (RetT->isLValueReferenceType()) {
-      ASTContext &Ctx = FD->getASTContext();
-      QualType LHST;
-      auto *MD = dyn_cast<CXXMethodDecl>(FD);
-      if (MD && MD->isCXXInstanceMember())
-        LHST = Ctx.getLValueReferenceType(MD->getFunctionObjectParameterType());
-      else
-        LHST = MD->getParamDecl(0)->getType();
-      if (Ctx.hasSameType(RetT, LHST))
-        return true;
-    }
-  }
-
-  return false;
+  // Track the implicit object parameter if this is a normal assignment operator.
+  return isNormalAssignmentOperator(FD);
 }
 
 static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index b3ca173c1fdbc..1057509137555 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -120,6 +120,7 @@ MyLongPointerFromConversion global2;
 
 void initLocalGslPtrWithTempOwner() {
   MyIntPointer p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  MyIntPointer pp = p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   p = MyIntOwner{}; // TODO ?
   global = MyIntOwner{}; // TODO ?
   MyLongPointerFromConversion p2 = MyLongOwnerWithConversion{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff c3079ffcd34e8ee2faaf7576a69a49acc1a3653f e1bd3e3d813ecce1e93957c3cbe26a8c3182abd6 -- clang/lib/Sema/CheckExprLifetime.cpp clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index f2fd3849b0..d30d17d461 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -325,7 +325,7 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
 // Assume that all assignment operators with a "normal" return type return
 // *this, that is, an lvalue reference that is the same type as the implicit
 // object parameter (or the LHS for a non-member operator$=).
-static bool isNormalAssignmentOperator(const FunctionDecl* FD) {
+static bool isNormalAssignmentOperator(const FunctionDecl *FD) {
   if (!FD)
     return false;
   OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
@@ -346,7 +346,6 @@ static bool isNormalAssignmentOperator(const FunctionDecl* FD) {
   return false;
 }
 
-
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
                                     LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
@@ -441,7 +440,8 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
       return true;
   }
 
-  // Track the implicit object parameter if this is a normal assignment operator.
+  // Track the implicit object parameter if this is a normal assignment
+  // operator.
   return isNormalAssignmentOperator(FD);
 }
 

[hokein]

Withdrawing it from review now. I seem to find a potentially better approach and will upload a revised version.

[hokein]

The feature has already been implemented in another pull request, so I will close this one now.