Skip to content

Implement arbitrary-self dynamic method receivers #1038

New issue

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

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

Already on GitHub? # to your account

Closed
RalfJung opened this issue Nov 7, 2019 · 8 comments · Fixed by rust-lang/rust#95071
Closed

Implement arbitrary-self dynamic method receivers #1038

RalfJung opened this issue Nov 7, 2019 · 8 comments · Fixed by rust-lang/rust#95071
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE S-blocked Status: blocked on something happening somewhere else

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 7, 2019

The tests added in rust-lang/rust#54383 fail in Miri:

  15: core::result::unwrap_failed
             at src/libcore/result.rs:1165
  16: core::result::Result<T,E>::unwrap
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/libcore/result.rs:933
  17: rustc_mir::interpret::place::<impl rustc_mir::interpret::operand::OpTy<Tag>>::assert_mem_place
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/place.rs:232
  18: rustc_mir::interpret::terminator::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_fn_call
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/terminator.rs:437
  19: rustc_mir::interpret::terminator::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_terminator
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/terminator.rs:94
  20: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::terminator
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/step.rs:293
  21: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::step
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/step.rs:69
  22: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::run
             at /rustc/e4931eaaa3d95189b30e90d3af9f0db17c41bbb0/src/librustc_mir/interpret/step.rs:40
  23: miri::eval::eval_main::{{closure}}
             at src/eval.rs:190

This is ICEing in this code:

https://github.com/rust-lang/rust/blob/7a76fe76f756895b8cda1e10398f2268656a2e0f/src/librustc_mir/interpret/terminator.rs#L430-L438

Clearly, that code assumes that only things that builtin_deref supports are receivers, and everything else is unsized -- hence the assert_mem_place, which fails.

So, how does one implement arbitrary-self dynamic receivers? What actually happens at run-time here? I don't know.^^

Cc @eddyb @mikeyhew

@RalfJung RalfJung added A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. labels Nov 7, 2019
@eddyb
Copy link
Member

eddyb commented Nov 7, 2019

This is one of the things we have to do (you'd get this for free if you're using FnAbi in miri):
https://github.com/rust-lang/rust/blob/50f8aadd746ebc929a752e5ffb133936ee75c52f/src/librustc/ty/layout.rs#L2550-L2572

@RalfJung
Copy link
Member Author

RalfJung commented Nov 7, 2019

you'd get this for free if you're using FnAbi in miri

You keep saying that.^^ Is that type in a place these days where Miri could reuse it? I recall it used to be in the wrong crate.

Also I have no idea what it would take to port Miri to that API.

@eddyb
Copy link
Member

eddyb commented Nov 7, 2019

I guess you might want to wait for rust-lang/rust#65947 to make it clearer how you're supposed to create it.
But yeah it's been in librustc for a while now.

@RalfJung
Copy link
Member Author

IOW, this is blocked on rust-lang/rust#56166.

@RalfJung
Copy link
Member Author

My understanding was that FnAbi would take care of the details of this, but now I found this code:

https://github.com/rust-lang/rust/blob/50ec47aa06e96a195707fb5ee92fcd32299ca272/compiler/rustc_codegen_ssa/src/mir/block.rs#L732-L747

Looks like the codegen backends and Miri still need some ad-hoc code to descend to the right type of an arbitrary-self-receiver.

Similar code exists at

https://github.com/rust-lang/rust/blob/767471edebd292d7b6386ed236e82abef9abc330/compiler/rustc_middle/src/ty/layout.rs#L3237

I am not sure if that is exactly the same or subtly different, but I feel like some kind of helper method could at least share the code between codegen and Miri -- but I don't understand enough to figure out the details.

@bjorn3
Copy link
Member

bjorn3 commented Nov 28, 2021

The basic idea is that if you have a fat pointer you need to strip the vtable pointer to make it a thin pointer before passing it as self value to the callee. Both snippets you linked unwrap a fat pointer embedded inside a struct so that for struct Box<T>(*mut T) or struct Rc<T>(*mut RcBox<T>) the inner fat pointer is still made thin beforr passing it as argument.

@RalfJung
Copy link
Member Author

But the entire type is just a newtyped fat ptr, right? So really this is just type information getting in the way and what we would want is to just transmute the argument to a data ptr and a vtable ptr and handle those separately?

@RalfJung
Copy link
Member Author

Also I don't think I understand how using FnAbi (implemented in rust-lang/rust#91342) should help with this... now that I did the FnAbi change, it seems mostly orthogonal to me.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE S-blocked Status: blocked on something happening somewhere else
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants