-
Notifications
You must be signed in to change notification settings - Fork 8
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
Tests fails under miri #21
Comments
Miri fails because Smallbox employs a technique called "pointer-usize-pointer roundtrips", which cannot be expressed within the constraints of Strict Provenance. This doesn't necessarily indicate that the code has undefined behavior, but it means that tools like MIRI cannot prove that it doesn't. The changes required to comply with Strict Provenance rules are currently under the nightly feature gate |
I don’t think ptr-int-ptr roundtrips are the only problem here. Consider this patch: Patchdiff --git a/src/smallbox.rs b/src/smallbox.rs
index 499dd7b..8308f99 100644
--- a/src/smallbox.rs
+++ b/src/smallbox.rs
@@ -182,8 +182,8 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {
// Overwrite the pointer but retain any extra data inside the fat pointer.
let mut ptr = ptr;
- let ptr_ptr = &mut ptr as *mut _ as *mut usize;
- ptr_ptr.write(ptr_addr as usize);
+ let ptr_ptr = &mut ptr as *mut _ as *mut *const u8;
+ ptr_ptr.write(ptr_addr);
ptr::copy_nonoverlapping(val as *const _ as *const u8, ptr_copy, size);
@@ -221,11 +221,9 @@ impl<T: ?Sized, Space> SmallBox<T, Space> {
unsafe fn as_ptr(&self) -> *const T {
let mut ptr = self.ptr;
- if !self.is_heap() {
- // Overwrite the pointer but retain any extra data inside the fat pointer.
- let ptr_ptr = &mut ptr as *mut _ as *mut usize;
- ptr_ptr.write(self.space.as_ptr() as *const () as usize);
- }
+ // Overwrite the pointer but retain any extra data inside the fat pointer.
+ let ptr_ptr = &mut ptr as *mut *const T as *mut *const Space;
+ ptr_ptr.write(self.space.as_ptr());
ptr
} It gets rid of such roundtrips and tests still fail under Miri, with both Stacked Borrows and Tree Borrows. Error with Stacked Borrows
Error with Tree Borrows
Interestingly, Miri with Tree Borrows mentions reading uninitialized memory, which is certainly UB, regardless of which borrowing model will be adopted at the end. I haven’t been able to pinpoint why this happens though, and it might be a bug in Miri. Anyway, I think that deserves further investigation. |
@GoldsteinE I believe that this patch doesn't satisfy the 'strict provenance' requirement, which necessitates manipulating the pointer using the API associated with #[feature(strict_provenance)]. Otherwise, information about the memory to which the pointer is directed will be lost in the miri context (when built with miri, the pointers do carry extra information about the memory, and the strict_provenance API preserves that information). |
Strict provenance requirements aren’t checked by Miri by default AFAIK, so SB/TB failures can’t be caused by them. Miri treats |
I’m also not sure which strict provenance APIs you’re talking about. It seems like there’s no roundtrip at all, so no special APIs are needed? |
Yeah, I know talk is cheap. Therefore, I tested the new feature-gated ptr APIs and as expected, Miri passed without any issues. |
That’s cool! Does it work with |
It needs an additional |
I also feel like “works with strict provenance APIs, but not ptr-to-ptr casts” could be a bug in Miri. Maybe it’s worth reporting this case to Miri and/or UCG. |
In your patch, the address of ptr is still directly derived from an usize
|
Oops, fair enough. I’ll try to make a working patch without sptr APIs later today. |
Oh my bad, |
@andylokandy Could you provide your sptr patch as a file? I’ll try to tinker with it some more. |
@GoldsteinE You can see this #22 |
It seems as miri test is broken. Could you please investigate it ?
The text was updated successfully, but these errors were encountered: