Skip to content

Commit 2fee9de

Browse files
committed
miri: make vtable addresses not globally unique
1 parent c4352e9 commit 2fee9de

File tree

5 files changed

+57
-7
lines changed

5 files changed

+57
-7
lines changed

src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ extern crate either;
5656
extern crate tracing;
5757

5858
// The rustc crates we need
59+
extern crate rustc_attr;
5960
extern crate rustc_apfloat;
6061
extern crate rustc_ast;
6162
extern crate rustc_const_eval;

src/machine.rs

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rand::rngs::StdRng;
1111
use rand::Rng;
1212
use rand::SeedableRng;
1313

14+
use rustc_attr::InlineAttr;
1415
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1516
#[allow(unused)]
1617
use rustc_data_structures::static_assert_size;
@@ -47,10 +48,10 @@ pub const SIGRTMIN: i32 = 34;
4748
/// `SIGRTMAX` - `SIGRTMIN` >= 8 (which is the value of `_POSIX_RTSIG_MAX`)
4849
pub const SIGRTMAX: i32 = 42;
4950

50-
/// Each const has multiple addresses, but only this many. Since const allocations are never
51-
/// deallocated, choosing a new [`AllocId`] and thus base address for each evaluation would
52-
/// produce unbounded memory usage.
53-
const ADDRS_PER_CONST: usize = 16;
51+
/// Each anonymous global (constant, vtable, function pointer, ...) has multiple addresses, but only
52+
/// this many. Since const allocations are never deallocated, choosing a new [`AllocId`] and thus
53+
/// base address for each evaluation would produce unbounded memory usage.
54+
const ADDRS_PER_ANON_GLOBAL: usize = 32;
5455

5556
/// Extra data stored with each stack frame
5657
pub struct FrameExtra<'tcx> {
@@ -1372,7 +1373,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
13721373
catch_unwind: None,
13731374
timing,
13741375
is_user_relevant: ecx.machine.is_user_relevant(&frame),
1375-
salt: ecx.machine.rng.borrow_mut().gen::<usize>() % ADDRS_PER_CONST,
1376+
salt: ecx.machine.rng.borrow_mut().gen::<usize>() % ADDRS_PER_ANON_GLOBAL,
13761377
};
13771378

13781379
Ok(frame.with_extra(extra))
@@ -1518,4 +1519,40 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
15181519
Entry::Occupied(oe) => Ok(oe.get().clone()),
15191520
}
15201521
}
1522+
1523+
fn get_global_alloc_salt(
1524+
ecx: &InterpCx<'tcx, Self>,
1525+
instance: Option<ty::Instance<'tcx>>,
1526+
) -> usize {
1527+
let unique = if let Some(instance) = instance {
1528+
// Functions cannot be identified by pointers, as asm-equal functions can get deduplicated
1529+
// by the linker (we set the "unnamed_addr" attribute for LLVM) and functions can be
1530+
// duplicated across crates. We thus generate a new `AllocId` for every mention of a
1531+
// function. This means that `main as fn() == main as fn()` is false, while `let x = main as
1532+
// fn(); x == x` is true. However, as a quality-of-life feature it can be useful to identify
1533+
// certain functions uniquely, e.g. for backtraces. So we identify whether codegen will
1534+
// actually emit duplicate functions. It does that when they have non-lifetime generics, or
1535+
// when they can be inlined. All other functions are given a unique address.
1536+
// This is not a stable guarantee! The `inline` attribute is a hint and cannot be relied
1537+
// upon for anything. But if we don't do this, backtraces look terrible.
1538+
let is_generic = instance
1539+
.args
1540+
.into_iter()
1541+
.any(|kind| !matches!(kind.unpack(), ty::GenericArgKind::Lifetime(_)));
1542+
let can_be_inlined = match ecx.tcx.codegen_fn_attrs(instance.def_id()).inline {
1543+
InlineAttr::Never => false,
1544+
_ => true,
1545+
};
1546+
!is_generic && !can_be_inlined
1547+
} else {
1548+
// Non-functions are never unique.
1549+
false
1550+
};
1551+
// Always use the same salt if the allocation is unique.
1552+
if unique {
1553+
CTFE_ALLOC_SALT
1554+
} else {
1555+
ecx.machine.rng.borrow_mut().gen::<usize>() % ADDRS_PER_ANON_GLOBAL
1556+
}
1557+
}
15211558
}

tests/pass/dyn-traits.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,17 @@ fn unsized_dyn_autoderef() {
141141
}
142142
*/
143143

144+
fn vtable_ptr_eq() {
145+
use std::{fmt, ptr};
146+
147+
// We don't always get the same vtable when casting this to a wide pointer.
148+
let x = &2;
149+
let x_wide = x as &dyn fmt::Display;
150+
assert!((0..256).any(|_| !ptr::eq(x as &dyn fmt::Display, x_wide)));
151+
}
152+
144153
fn main() {
145154
ref_box_dyn();
146155
box_box_trait();
156+
vtable_ptr_eq();
147157
}

tests/pass/function_pointers.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ fn main() {
8282
assert!(return_fn_ptr(i) == i);
8383
assert!(return_fn_ptr(i) as unsafe fn() -> i32 == i as fn() -> i32 as unsafe fn() -> i32);
8484
// Miri gives different addresses to different reifications of a generic function.
85-
assert!(return_fn_ptr(f) != f);
85+
// at least if we try often enough.
86+
assert!((0..256).any(|_| return_fn_ptr(f) != f));
8687
// However, if we only turn `f` into a function pointer and use that pointer,
8788
// it is equal to itself.
8889
let f2 = f as fn() -> i32;

tests/pass/rc.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ fn rc_fat_ptr_eq() {
7575
let p = Rc::new(1) as Rc<dyn Debug>;
7676
let a: *const dyn Debug = &*p;
7777
let r = Rc::into_raw(p);
78-
assert!(a == r);
78+
// Only compare the pointer parts, as the vtable might differ.
79+
assert!(a as *const () == r as *const ());
7980
drop(unsafe { Rc::from_raw(r) });
8081
}
8182

0 commit comments

Comments
 (0)