[clang] c725138 - [clang][Interp] Check pointers for live-ness when returning them

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-26T09:52:09+02:00
New Revision: c7251385c85ddcc1059548038e60099fcc5b304c

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

LOG: [clang][Interp] Check pointers for live-ness when returning them

We might be trying to return a pointer or reference to a local variable,
which doesn't work.

Differential Revision: https://reviews.llvm.org/D154795

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index e0cbf56e196958..4058d43c0bced5 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -181,6 +181,17 @@ template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   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.
+    if (!Ret.isLive())
+      return false;
+  }
+
   assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
   if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
     S.Current->popArgs();

diff  --git a/clang/test/AST/Interp/functions.cpp b/clang/test/AST/Interp/functions.cpp
index 629d0323e1d2e8..4bb8791de8f4e5 100644
--- a/clang/test/AST/Interp/functions.cpp
+++ b/clang/test/AST/Interp/functions.cpp
@@ -265,3 +265,29 @@ namespace CallWithArgs {
     g(0);
   }
 }
+
+namespace ReturnLocalPtr {
+  constexpr int *p() {
+    int a = 12;
+    return &a; // ref-warning {{address of stack memory}} \
+               // expected-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.
+  constexpr const int &p2() {
+    int a = 12; // ref-note {{declared here}}
+    return a; // ref-warning {{reference to stack memory associated with local variable}} \
+              // expected-warning {{reference to stack memory associated with local variable}}
+  }
+
+  static_assert(p2() == 12, ""); // ref-error {{not an integral constant expression}} \
+                                 // ref-note {{read of variable whose lifetime has ended}} \
+                                 // expected-error {{not an integral constant expression}}
+}