-
Notifications
You must be signed in to change notification settings - Fork 385
adjust Miri to Pointer type overhaul #1851
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
Conversation
25da527
to
3132d62
Compare
3efb5aa
to
b96b922
Compare
b96b922
to
a1233a7
Compare
I think the only user-visible change of this (aside from ptr-to-int casts working in a few more places) is more accurate overflow detection for pointer arithmetic. I will try to craft some testcases for that. |
Hm, I actually can't think of an overflow test that would error now where it passed before, or vice versa... we just get more accurate error messages now, which is already reflected in some of the test changes (but I might add one or two more). |
…oli-obk CTFE/Miri engine Pointer type overhaul This fixes the long-standing problem that we are using `Scalar` as a type to represent pointers that might be integer values (since they point to a ZST). The main problem is that with int-to-ptr casts, there are multiple ways to represent the same pointer as a `Scalar` and it is unclear if "normalization" (i.e., the cast) already happened or not. This leads to ugly methods like `force_mplace_ptr` and `force_op_ptr`. Another problem this solves is that in Miri, it would make a lot more sense to have the `Pointer::offset` field represent the full absolute address (instead of being relative to the `AllocId`). This means we can do ptr-to-int casts without access to any machine state, and it means that the overflow checks on pointer arithmetic are (finally!) accurate. To solve this, the `Pointer` type is made entirely parametric over the provenance, so that we can use `Pointer<AllocId>` inside `Scalar` but use `Pointer<Option<AllocId>>` when accessing memory (where `None` represents the case that we could not figure out an `AllocId`; in that case the `offset` is an absolute address). Moreover, the `Provenance` trait determines if a pointer with a given provenance can be cast to an integer by simply dropping the provenance. I hope this can be read commit-by-commit, but the first commit does the bulk of the work. It introduces some FIXMEs that are resolved later. Fixes rust-lang/miri#841 Miri PR: rust-lang/miri#1851 r? `@oli-obk`
@bors r+ |
📌 Commit 0d65d50 has been approved by |
☀️ Test successful - checks-actions |
This is the Miri side of rust-lang/rust#87123.
This was a lot more work than I expected... lucky enough it is also (hopefully) the last large-scale refactoring I will do.^^
Fixes #224