rust-lang
FnAbi Compatability check
I have made changes only in a single place to make sure that this what we want.
The right approach has to be somewhat similar to what happens for regular function calls in Miri. Basically, we need a helper function that takes a list of types (the expected argument types) and a return type, and then somehow checks that against the
FnAbithat is passed around in the Miri shim handling. This likely involved constructing our ownFnAbito compare, and I have no idea how to do that...
For non variadic fn's can't we just simply create FnAbi struct using the types available in docs. The issue only comes for variadic fn's as we dont know the types of the extra args.
Yes we "just" have to do that but it is not necessarily so easy. :)
But we can probably construct a fn ptr type corresponding to the desired signature, and then use fn_abi_of_fn_ptr.
Yes we "just" have to do that but it is not necessarily so easy. :)
Why can't we directly instantiate an object of FnAbi struct?
Also are you talking about this fn_abi_of_fn_ptr ?
Why can't we instantiate an object of FnAbi struct?
That involves computing a PassMode, and we most definitely do not want to compute those ourselves. We want to use the logic in the Rust compiler for computing those.
Also are you talking about this fn_abi_of_fn_ptr ?
Yes. It can be invoked in Miri via this.fn_abi_of_fn_ptr (where this is an InterpCx).
To resolve I was thinking of implementing the following things:
Create a fn check_fn_abi which would take a list of input types , an output type and Fnabi that is passed in the miri shim handling. This fn would create a FnSig using the args. Now we will create expected FnAbi using this fn sig and compare it with the input Fnsig. If it doesn't matches throw ub.
Now Transmute can be called safely inside shim.
That sounds like a good plan. There are methods defined in https://github.com/rust-lang/rust/blob/master/compiler/rustc_const_eval/src/interpret/call.rs that are probably useful but have to be made pub before they can be called from Miri.
There are methods defined in https://github.com/rust-lang/rust/blob/master/compiler/rustc_const_eval/src/interpret/call.rs that are probably useful but have to be made
pubbefore they can be called from Miri.
Are you talking about check_argument_compatmethod here? If yes now then I think we can compare the two FnAbi by iterating over the args of the two abis by calling check_argument_compat method.
Also inputs_and_output field of FnSig requires list as RawList<(), T> so I need to convert given input and output arg slice into RawList<(), T> using from_arena method?
Are you talking about check_argument_compatmethod here?
Yes.
Also inputs_and_output field of FnSig requires list as RawList<(), T> so I need to convert given input and output arg slice into RawList<(), T> using from_arena method?
You can use these methods: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/?search=mk_type_list.
I am mostly done with the code now I just need check_argument_compat to be public.
Okay, please make a PR against the rustc repo to make that function public then. :)
:umbrella: The latest upstream changes (possibly aec4fff9a88f594ce64f65abe93cc4bc5fc33561) made this pull request unmergeable. Please resolve the merge conflicts.
Please don't do rustup in the middle of a regular PR. I did a regular rustup, so if you rebase your PR should pick that up. :)
@rustbot author
Please post @rustbot ready when the PR is ready for the next round of review.
Hey, have some patience. I do this mostly in my free time. You're in the queue, and I will get to your PR eventually. Other people have been patiently waiting a lot more than 3 days.
This looks great, thanks! Please squash the commits, then we can land this. Please use the --keep-base flag when squashing so that the force-push diff is easier to review. Then write @rustbot ready after you pushed the squashed PR.
@rustbot author
So the next step would be to use check_shim_abi for non variadic shims? If yes I can work on that as well
Yeah the next step is to use this for all the existing shims. Please don't do them all in one PR. :) Pick one file, e.g. src/shims/foreign_items.rs, and make a PR just for that. In the first pass, skip any that cause some extra complications.