[clang] [clang] Extend lifetimebound analysis to detect within-initializer assignments between pointer-like objects. (PR #97473)

[Haojian Wu via cfe-commits]

https://github.com/hokein created https://github.com/llvm/llvm-project/pull/97473

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

>From e1bd3e3d813ecce1e93957c3cbe26a8c3182abd6 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 2 Jul 2024 13:28:37 +0200
Subject: [PATCH] [clang] Extend lifetimebound analysis to detect
 within-initializer assignments between pointer-like objects.

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
---
 clang/lib/Sema/CheckExprLifetime.cpp          | 69 +++++++++++++------
 .../Sema/warn-lifetime-analysis-nocfg.cpp     |  1 +
 2 files changed, 50 insertions(+), 20 deletions(-)

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}}