[clang] 056cd12 - [clang][bytecode] Don't check returned pointers for liveness (#120107)

[via cfe-commits]

Author: Timm Baeder
Date: 2024-12-17T06:20:14+01:00
New Revision: 056cd12284f72105a3e2338f901882e43a90c8b2

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

LOG: [clang][bytecode] Don't check returned pointers for liveness (#120107)

We're supposed to let them through and then later diagnose reading from
them, but returning dead pointers is fine.

Added: 
    

Modified: 
    clang/lib/AST/ByteCode/Interp.h
    clang/test/AST/ByteCode/functions.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f085f96fdf5391..2a6ea69475f787 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -318,18 +318,6 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool Ret(InterpState &S, CodePtr &PC) {
   const T &Ret = S.Stk.pop<T>();
 
-  // Make sure returned pointers are live. We might be trying to return a
-  // pointer or reference to a local variable.
-  // Just return false, since a diagnostic has already been emitted in Sema.
-  if constexpr (std::is_same_v<T, Pointer>) {
-    // FIXME: We could be calling isLive() here, but the emitted diagnostics
-    // seem a little weird, at least if the returned expression is of
-    // pointer type.
-    // Null pointers are considered live here.
-    if (!Ret.isZero() && !Ret.isLive())
-      return false;
-  }
-
   assert(S.Current);
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller)

diff  --git a/clang/test/AST/ByteCode/functions.cpp b/clang/test/AST/ByteCode/functions.cpp
index b00f59a8d8d433..10bea3a0d017e2 100644
--- a/clang/test/AST/ByteCode/functions.cpp
+++ b/clang/test/AST/ByteCode/functions.cpp
@@ -303,21 +303,17 @@ namespace ReturnLocalPtr {
     return &a; // both-warning {{address of stack memory}}
   }
 
-  /// GCC rejects the expression below, just like the new interpreter. The current interpreter
-  /// however accepts it and only warns about the function above returning an address to stack
-  /// memory. If we change the condition to 'p() != nullptr', it even succeeds.
-  static_assert(p() == nullptr, ""); // ref-error {{static assertion failed}} \
-                                     // expected-error {{not an integral constant expression}}
-
-  /// FIXME: The current interpreter emits diagnostics in the reference case below, but the
-  /// new one does not.
+  /// FIXME: Both interpreters should diagnose this. We're returning a pointer to a local
+  /// variable.
+  static_assert(p() == nullptr, ""); // both-error {{static assertion failed}}
+
   constexpr const int &p2() {
-    int a = 12; // ref-note {{declared here}}
+    int a = 12; // both-note {{declared here}}
     return a; // both-warning {{reference to stack memory associated with local variable}}
   }
 
   static_assert(p2() == 12, ""); // both-error {{not an integral constant expression}} \
-                                 // ref-note {{read of variable whose lifetime has ended}}
+                                 // both-note {{read of variable whose lifetime has ended}}
 }
 
 namespace VoidReturn {