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

Shims ICE when a scalar argument isn't actually a scalar #3842

Open
RalfJung opened this issue Aug 24, 2024 · 2 comments
Open

Shims ICE when a scalar argument isn't actually a scalar #3842

RalfJung opened this issue Aug 24, 2024 · 2 comments
Labels
A-shims Area: This affects the external function shims C-bug Category: This is a bug. E-good-second-issue A good issue to pick up if you've already seen some parts of Miri, mentoring is available I-ICE Impact: makes Miri crash with some ICE

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2024

For instance, consider:

extern "C" {
    fn pipe(pipefd: [i32; 2]) -> i32;
}

fn main() {
    let mut fds: [i32; 2] = [0; 2]; 
    assert_eq!(unsafe { pipe(fds) }, 0); 
}

I think to fix this properly we need to completely re-do the way we handle shim arguments: every OpTy needs to be transmuted to the right type before we do anything else with it. And we can't just just call OpTy::transmute as that can't be used for arbitrary transmute, e.g. changing ABIs can fail spectacularly.

Once this is consistently done everywhere, we can also remove the ScalarSizeMismatch error from the interpreter. This was added because otherwise ICEs can be triggered by using an incorrect scalar type in the signature of a function that has a Miri shim, but as we have seen, ICEs can still be triggered -- and once we use our own hard-coded types everywhere, ScalarSizeMismatch can no longer be triggered at all.

We probably want a macro that helps with that.

@RalfJung RalfJung added C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE E-good-second-issue A good issue to pick up if you've already seen some parts of Miri, mentoring is available A-shims Area: This affects the external function shims labels Aug 24, 2024
@RalfJung
Copy link
Member Author

I think we want a new function like get_arg or so that preprocesses the OpTy. Given an op: OpTy and ty: Ty, we do a check like

  • let layout = layout_of(ty);
  • Check layout.abi.eq_up_to_validity(&op.layout.abi). If that is false, the argument type is invalid -> raise UB.
  • Return op.transmute(layout).

@RalfJung
Copy link
Member Author

RalfJung commented Sep 29, 2024

Similar ICEs can occur when the return type of the function is wrong -- we end up calling write_scalar with a scalar of the wrong size. This either fails to initialize a part of the result (if the scalar is smaller than the return place), or ICEs since we are exceeding the bounds of our AllocRefMut (if the scalar is bigger than the return place).

If it is a ScalarPair, we can even end up writing things at wrong offsets. (But AllocRefMut will at least keep us inside the bounds of the place.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-bug Category: This is a bug. E-good-second-issue A good issue to pick up if you've already seen some parts of Miri, mentoring is available I-ICE Impact: makes Miri crash with some ICE
Projects
None yet
Development

No branches or pull requests

1 participant