-
Notifications
You must be signed in to change notification settings - Fork 278
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
Make strict provenance compatible #542
Conversation
src/bytes.rs
Outdated
fn ptr_map<F>(ptr: *mut u8, f: F) -> *mut u8 | ||
where | ||
F: FnOnce(usize) -> usize, | ||
{ | ||
let old_addr = ptr as usize; | ||
let new_addr = f(old_addr); | ||
let diff = new_addr.wrapping_sub(old_addr); | ||
ptr.wrapping_add(diff) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it seems like this doesn't really optimize as well as I had hoped. You can view godbolt here to see the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, that's kind of sad...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One option is to go through a normal pointer cast for non-miri release mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this valid? This seems to optimize well, though I have no idea why. I just tried it out because I find this pattern for writing pointer packing more intuitive.
https://rust.godbolt.org/z/a949Wjs7f (I previously left a target-cpu
in the flags, oops)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good to me!
FYI, the optimization you do to avoid the bad codegen causes (non-theoretical) LLVM miscompiles -- See rust-lang/rust#96538 for some details. So I don't know if you should keep it. |
welp. that's cool. |
@thomcc Thanks. We will uses ptr2int2ptr casts for now on non-miri builds then. |
This PR replaces all ptr2int2ptr casts with an
wrapping_add
equivalent, making it strict-provenance compatible.