Skip to content

[clang][bytecode] Don't check returned pointers for liveness #120107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions clang/lib/AST/ByteCode/Interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 6 additions & 10 deletions clang/test/AST/ByteCode/functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading