Skip to content
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

Tail calls end protectors too early #3788

Closed
RalfJung opened this issue Aug 5, 2024 · 2 comments
Closed

Tail calls end protectors too early #3788

RalfJung opened this issue Aug 5, 2024 · 2 comments
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) C-spec-question Category: it is unclear what the intended behavior of Miri for this case is

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 5, 2024

In a tail call, before_stack_pop gets called before the stack frame is replaced with that of the new function. This ends the corresponding protectors in Miri, which is premature: they should instead be carried over to the new stack frame, and only ended when we return back to the caller.

This affects both protection of function arguments and, perhaps even more relevant, the return place.

Currently it is hard to write a test case since custom MIR does not yet support tail calls (support is being added in rust-lang/rust#128688).

@RalfJung RalfJung added C-bug Category: This is a bug. A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) labels Aug 5, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Aug 6, 2024

Ah, actually it seems like we do handle this properly for the return place -- a new protector gets created for the same return place again.

However, function arguments do not get re-protected unless they are passed to the new function. So there's a question here whether the scope of a "noalias" function argument extends until just before a tail call, or until after the tail call returns.

@RalfJung RalfJung added C-spec-question Category: it is unclear what the intended behavior of Miri for this case is and removed C-bug Category: This is a bug. I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) labels Aug 6, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Aug 6, 2024

Closing in favor of rust-lang/unsafe-code-guidelines#523.

@RalfJung RalfJung closed this as completed Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) C-spec-question Category: it is unclear what the intended behavior of Miri for this case is
Projects
None yet
Development

No branches or pull requests

1 participant