Skip to content

Commit

Permalink
vtable: fix UB detected as miri errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ogoffart committed Feb 21, 2024
1 parent a53c4c8 commit 5605c60
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 21 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,15 @@ jobs:
sed -i "s/#wasm# //" Cargo.toml
cargo apk build -p printerdemo --target aarch64-linux-android --lib
working-directory: examples/printerdemo/rust

miri:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/setup-rust
with:
toolchain: nightly
key: miri
components: miri
- name: Run Miri
run: cargo miri test -p vtable -p const-field-offset -p i-slint-common
4 changes: 2 additions & 2 deletions helper_crates/vtable/macro/macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,8 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
// Safety: The vtable is valid and inner is a type corresponding to the vtable,
// which was allocated such that drop is expected.
unsafe {
let ptr = &*ptr;
(ptr.vtable.as_ref().#ident)(VRefMut::from_raw(ptr.vtable, ptr.ptr)) }
let (vtable, ptr) = ((*ptr).vtable, (*ptr).ptr);
(vtable.as_ref().#ident)(VRefMut::from_raw(vtable, ptr)) }
}
fn new_box<X: HasStaticVTable<#vtable_name>>(value: X) -> VBox<#vtable_name> {
// Put the object on the heap and get a pointer to it
Expand Down
40 changes: 21 additions & 19 deletions helper_crates/vtable/src/vrc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,8 @@ struct VRcInner<'vt, VTable: VTableMeta, X> {
}

impl<'vt, VTable: VTableMeta, X> VRcInner<'vt, VTable, X> {
fn data_ptr(&self) -> *const X {
let ptr = self as *const Self as *const u8;
unsafe { ptr.add(self.data_offset as usize) as *const X }
unsafe fn data_ptr(s: *const Self) -> *const X {
(s as *const u8).add(*core::ptr::addr_of!((*s).data_offset) as usize) as *const X
}
fn as_ref(&self) -> &X {
let ptr = self as *const Self as *const u8;
Expand All @@ -102,25 +101,27 @@ pub struct VRc<VTable: VTableMetaDropInPlace + 'static, X = Dyn> {
impl<VTable: VTableMetaDropInPlace + 'static, X> Drop for VRc<VTable, X> {
fn drop(&mut self) {
unsafe {
let inner = self.inner.as_ref();
if inner.strong_ref.fetch_sub(1, Ordering::SeqCst) == 1 {
let data = inner.data_ptr() as *const _ as *const u8 as *mut u8;
let mut layout = VTable::drop_in_place(inner.vtable, data);
let inner = self.inner.as_ptr();
if (*inner).strong_ref.fetch_sub(1, Ordering::SeqCst) == 1 {
let data =
(inner as *mut u8).add(*core::ptr::addr_of!((*inner).data_offset) as usize);
let vtable = core::ptr::addr_of!((*inner).vtable);
let mut layout = VTable::drop_in_place(&*vtable, data);
layout = core::alloc::Layout::new::<VRcInner<VTable, ()>>()
.extend(layout.try_into().unwrap())
.unwrap()
.0
.pad_to_align()
.into();
if inner.weak_ref.load(Ordering::SeqCst) > 1 {
if (*core::ptr::addr_of!((*inner).weak_ref)).load(Ordering::SeqCst) > 1 {
// at this point we are sure that no other thread can access the data
// since we still hold a weak reference, so the other weak references
// in other thread won't start destroying the object.
self.inner.cast::<VRcInner<VTable, Layout>>().as_mut().data = layout;
*(VRcInner::data_ptr(self.inner.cast::<VRcInner<VTable, Layout>>().as_ptr())
as *mut Layout) = layout;
}
if inner.weak_ref.fetch_sub(1, Ordering::SeqCst) == 1 {
let vtable = inner.vtable;
VTable::dealloc(vtable, self.inner.cast().as_ptr(), layout);
if (*core::ptr::addr_of!((*inner).weak_ref)).fetch_sub(1, Ordering::SeqCst) == 1 {
VTable::dealloc(&*vtable, self.inner.cast().as_ptr(), layout);
}
}
}
Expand Down Expand Up @@ -220,8 +221,8 @@ impl<VTable: VTableMetaDropInPlace, X> VRc<VTable, X> {
unsafe {
let inner = this.inner.cast::<VRcInner<VTable, u8>>();
VRef::from_raw(
NonNull::from(inner.as_ref().vtable),
NonNull::from(&*inner.as_ref().data_ptr()),
NonNull::from(*::core::ptr::addr_of!((*inner.as_ptr()).vtable)),
NonNull::new_unchecked(VRcInner::data_ptr(inner.as_ptr()) as *mut u8),
)
}
}
Expand Down Expand Up @@ -303,13 +304,14 @@ impl<VTable: VTableMetaDropInPlace + 'static, X> Clone for VWeak<VTable, X> {
impl<T: VTableMetaDropInPlace + 'static, X> Drop for VWeak<T, X> {
fn drop(&mut self) {
if let Some(i) = self.inner {
let inner = unsafe { i.as_ref() };
if inner.weak_ref.fetch_sub(1, Ordering::SeqCst) == 1 {
let vtable = inner.vtable;
unsafe {
unsafe {
if (*core::ptr::addr_of!((*i.as_ptr()).weak_ref)).fetch_sub(1, Ordering::SeqCst)
== 1
{
// Safety: while allocating, we made sure that the size was big enough to
// hold a VRcInner<T, Layout>.
let layout = i.cast::<VRcInner<T, Layout>>().as_ref().data;
let vtable = &*core::ptr::addr_of!((*i.as_ptr()).vtable);
let layout = *(VRcInner::data_ptr(i.cast::<VRcInner<T, Layout>>().as_ptr()));
T::dealloc(vtable, i.cast().as_ptr(), layout);
}
}
Expand Down

0 comments on commit 5605c60

Please sign in to comment.