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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
