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

vtable: fix UB detected as miri errors #4646

Merged
merged 1 commit into from
Feb 21, 2024
Merged

vtable: fix UB detected as miri errors #4646

merged 1 commit into from
Feb 21, 2024

Conversation

ogoffart
Copy link
Member

We can't create references for things that aren't represented by the reference. Even if we never dereference that reference.

Also add a miri test in the CI that runs the test on crate which are error free.

We can't create references for things that aren't represented by the
reference. Even if we never dereference that reference.

Also add a miri test in the CI that runs the test on crate which are
error free.
}
if inner.weak_ref.fetch_sub(1, Ordering::SeqCst) == 1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m curious: what’s the intermediate reference that’s created where the types don’t match?

Copy link
Member

@tronical tronical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I obviously trust you on this, I understand also the premise of the change, but I don’t understand yet where we create the wrong reference, temporarily. (Asking for a future $me writing pointer code)

@ogoffart
Copy link
Member Author

What we create is a VRcInner<VTable, T> where T is some specific type that implement the trait for the VTable. So we should be careful not to take reference to VRcInner<VTable, Dyn> or VRcInner<VTable, Layout>, which are used to get the metadata of dynamic VRc or destroyed VRc, respectively, but are not "real"

@ogoffart ogoffart merged commit 5605c60 into master Feb 21, 2024
36 checks passed
@ogoffart ogoffart deleted the olivier/vtable branch February 21, 2024 07:12
@tronical
Copy link
Member

Thanks, makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants