native-lib: support all types with Scalar layout

[RalfJung]

Fixes #4625

However, this also means we'll silently just pass function pointers to C and things will explode in horrendous ways if C then ends up calling such a function pointer. So I don't think we can land this as-is. I am not sure what else to do... we could have a flag for people to opt-in to this and promise the function pointers will never be called?

[nia-e]

Could we detect if a pointer is a function pointer? If so, we can change its address (if non-null) to point to some preallocated single page of memory mapped R-X which raises a signal that the supervisor can intercept & print an error

[RalfJung]

We can't just change the value when it is being passed to C -- the program could realize this with some ptr2int casts. (Also, for fields in structs/unions behind ptr indirections, determining their type becomes more of a heuristic.)

However, we could adjust the logic that determines the addresses of function pointer allocations to begin with, to put them all onto some R-X page. Interesting idea.

[oli-obk]

Since we need to convert values into their FFI repr, could we reject all those with function provenance and error?

I guess a fully ptr2int converted function pointer would still get missed.

[RalfJung]

Our conversion function isn't type-driven -- it just takes the raw bytes of the argument values and sends them over.

We could scan that value and all memory reachable from it for function pointers, but currently we don't do such a recursive traversal so... not sure how I feel about that. If we can pull of the R-X page, that feels more slick (and it should reliably segfault even if we don't have the supervisor that can give a nice error).

EDIT: Actually, we do have the visit_reachable_allocs scan. That would then error even for function pointers that aren't actually used. Hm...

[nia-e]

Do we care about function pointers having different addresses? If not then we can do this even more simply, just have

pub extern "C" fn one_pointee_to_rule_them_all() {
    panic!("unsupported operation: native code tried to call a Rust function");
}

where all function pointers take its address.

[RalfJung]

yeah every function pointer needs to have a unique address for Miri itself to be able to handle them properly.

[nia-e]

Then we can have that function delcared somewhere, allocate as many R-X pages as needed, fill them with jmp one_pointee_to_rule_them_all calls, and get page_size / sizeof(jmp) distinct addresses per page :D

[RalfJung]

That still needs platform-specific code to generate the assembly. So we still need fallback code that creates an R-- page where just jumping there segfaults (which I assume can be done with mmap).

---

Anyway turns out Oli's suggestion is trivial to implement since we already have the visit_reachable_allocs scan; I pushed that. However, this does mean that if any function pointer gets cast to an integer ever, one can't make any more FFI calls... so maybe this shouldn't stop execution, and should just emit a warning saying "if you ever call that function, glhf"?

I assume that mallocd memory is non-executable everywhere these days, so calling such a function pointer should reliably crash and not execute garbage instructions.

[nia-e]

Fair, Oli's is probably the reasonable approach. We could do the other you suggested; there are "weird" architectures where R implies X (iirc m68k has this?) but if you're somehow running Miri on that it's likely fair to say that's a you problem

[rustbot]

Thank you for contributing to Miri!
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

[RalfJung]

Okay let's do that. :D

Interestingly, when I do actually call that function, the program just hangs. I think there's a segfault and then the supervisor is getting confused since the segfault is indeed in the address range it cares about...

[nia-e]

I assume this is the issue, that we're not checking that the segfault is cleared?

// in handle_segfault
-    let _ = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecEnd(None))?;
+    let stat = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecEnd(None))?;
+    match stat {
+        wait::WaitStatus::Signaled(_, s, _) | wait::WaitStatus::Stopped(_, s) | wait::WaitStatus::PtraceEvent(_, s, _) => assert!(!matches!(s, signal::SIGSEGV), "Native code segfaulted in correctly allocated memory; this is probably a function call!"),
+        _ => (),
+    }

[RalfJung]

That works well, thanks :)