[clang] a6d26c5 - [clang] Fix dangling false positives for conditional operators. (#120233)

[via cfe-commits]

Author: Haojian Wu
Date: 2024-12-20T09:26:38+01:00
New Revision: a6d26c56ff066c8e8f92f4ca169fcf40ae0db537

URL: https://github.com/llvm/llvm-project/commit/a6d26c56ff066c8e8f92f4ca169fcf40ae0db537
DIFF: https://github.com/llvm/llvm-project/commit/a6d26c56ff066c8e8f92f4ca169fcf40ae0db537.diff

LOG: [clang] Fix dangling false positives for conditional operators. (#120233)

When analyzing a dangling gsl pointer, we currently filter out all field
access `MemberExpr` to avoid common false positives (`string_view sv =
Temp().sv`), However, this filter only applies to direct MemberExpr
instances, leaving the conditional operator as an escaping example
(`GSLPointer pointer(Cond ? Owner().ptr : GSLPointer());`).

This patch extends the MemberExpr logic to handle the conditional
operator. The heuristic is intentionally simple, which may result in
some false negatives. However, it effectively covers common cases like
`std::string_view sv = cond ? "123" : std::string();`, which is a
reasonable trade-off.

Fixes https://github.com/llvm/llvm-project/issues/120206

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/CheckExprLifetime.cpp
    clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b8d92a6c881c68..a85ef60b7b58ba 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -706,6 +706,8 @@ Improvements to Clang's diagnostics
       return ptr + index < ptr; // warning
     }
 
+- Fix -Wdangling false positives on conditional operators (#120206).
+
 Improvements to Clang's time-trace
 ----------------------------------
 

diff  --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index add6d7506bd6f0..7109de03cadd12 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -582,6 +582,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
     //   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

diff  --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 45b4dc838f44ed..4c19367bb7f3dd 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -777,3 +777,32 @@ void test4() {
 }
 
 } // namespace LifetimeboundInterleave
+
+namespace GH120206 {
+struct S {
+  std::string_view s;
+};
+
+struct [[gsl::Owner]] Q1 {
+  const S* get() const [[clang::lifetimebound]];
+};
+std::string_view test1(int c, std::string_view sv) {
+  std::string_view k = c > 1 ? Q1().get()->s : sv;
+  if (c == 1)
+    return  c > 1 ? Q1().get()->s : sv;
+  Q1 q;
+  return c > 1 ? q.get()->s : sv;
+}
+
+struct Q2 {
+  const S* get() const [[clang::lifetimebound]];
+};
+std::string_view test2(int c, std::string_view sv) {
+  std::string_view k = c > 1 ? Q2().get()->s : sv;
+  if (c == 1)
+    return c > 1 ? Q2().get()->s : sv;
+  Q2 q;
+  return c > 1 ? q.get()->s : sv;
+}
+
+} // namespace GH120206