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

Support for passing const Miri pointers to C #2498

Closed
wants to merge 26 commits into from

Conversation

emarteca
Copy link

Supporting for passing const Miri pointers to C.

Fork of rustc required

This relies on the alignment of the bytes of Allocations to match the align parameter, and for the address of these bytes to be accessible to Miri: you need to run this with the version of rustc in this PR for it to work.

Main changes to Miri

This support builds on previous support for calling C functions with integer arguments, in this PR to Miri.
All the changes in that PR are contained here.

The changes to Miri since integer function support are mostly to intptrcast:

  • Move the generation of a new (fake) address to it's own function, get_next_fake_addr.
    We still need this functionality for cases where an alloc_id doesn't have a corresponding base address, or if we're running Miri in non-FFI mode and want to keep the same behaviour as before.
  • If we are in FFI mode and there is a base address accessible through ecx (via this function, in the fork of rustc), then we use this as the base address of the allocation (linked code)
  • Since we're now using the real addresses as the base_addr, using a Vec as the data structure for global_state.int_to_ptr_map is not efficient anymore, since it no longer is guaranteed to be sorted by just adding new addresses to the end of the list.
    So we changed the data structure to a BTreeMap, which is always sorted and provides efficient insertion.
    This change mostly shows up here in the code.
    Note: even though in non-FFI mode we use the original fake address functionality, we keep the BTreeMap.

We also changed ffi_support, adding representations for the pointer types.

This section of our design doc has more details on our design and implementation of passing Miri pointers to C.
This doc also explains how we plan to support passing Miri mut pointers to C, and what we've done for adding support for passing C pointers back to Miri.

Testing

We added a test that calls a C function that double dereferences a C pointer and returns the value.

Specific code:
C function:

int double_deref(const int **p) {
  return **p;
}

Rust code calling the C function:

extern "C" {
    fn double_deref(x: *const *const i32) -> i32;
}
fn main() {
    unsafe {
        let base: i32 = 42;
        let base_p: *const i32 = &base as *const i32;
        let base_pp: *const *const i32 = &base_p as *const *const i32;
        assert_eq!(double_deref(base_pp), 42);
    }
}

Note: There are some tests in Miri’s test suite that fail if they’re executed using the real bytes of the Allocation as the address. All of the failures are because of allocation alignment assumptions being violated, and we don’t think they correspond to bugs in our implementation.
This was found when we were testing – the version of Miri we pushed only uses the real bytes for the address if we’re executing in the FFI mode, and so none of these tests are affected.
This is all explained, and the failing tests are listed out, in this section of our design doc.

Ellen Arteca and others added 26 commits July 18, 2022 20:35
… `hir` types, some refactoring/renaming of functions, and edits to the related test file hierarchy
… now need to make allocid the memory address
This shows how the base address, if it can be acquired, can be used to
call external C functions. There are numerous TODOs, including the big
one of coming up with a way to access the bytes address without the huge
hole punch I put in rustc.

This also requires that the bytes addresses in rustc become properly
aligned.

Double dereference passes, as do most tests. Of the few that remain,
some of them make questionable assumptions (which we might need some way
to retain or put behind an option for sanitization) like that it is
fundamentally possible for two allocations to end up next to each other
- this won't happen when we're using a real heap. The simd tests failing
  are concerning, but can be looked into.
…eeds to be fixed in rustc, but the issue is the alignment check isnt formulated correctly wrt real addrs
@RalfJung
Copy link
Member

I don't know when I will have time for a proper review (and anyway we should land the other one first^^), but just one note on terminology: I don't think "fake" addresses is a good term. They are no more fake than any other part of Miri; they are the real address that these allocations have inside the Abstract Machine.

If you need to distinguish them, something like 'machine_address' or so would be a lot better.

@RalfJung
Copy link
Member

I haven't seen activity on this PR or the rustc one (rust-lang/rust#100467) in a while so I am going to close this. @emarteca (or anyone else who is interested) feel free to pick this up again in the future if/when you have time!

@RalfJung RalfJung closed this Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked-on-rust Status: Blocked on landing a Rust PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants