miri: fix ICE with symbolic alignment check on extern static
This commit is contained in:
parent
b11fbfbf35
commit
25635b9a96
8 changed files with 76 additions and 27 deletions
|
@ -992,10 +992,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||||
// Complain about any other kind of error -- those are bad because we'd like to
|
// Complain about any other kind of error -- those are bad because we'd like to
|
||||||
// report them in a way that shows *where* in the value the issue lies.
|
// report them in a way that shows *where* in the value the issue lies.
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
bug!(
|
bug!("Unexpected error during validation: {}", self.format_error(err));
|
||||||
"Unexpected Undefined Behavior error during validation: {}",
|
|
||||||
self.format_error(err)
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -383,7 +383,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
|
||||||
// will never cause UB on the pointer itself.
|
// will never cause UB on the pointer itself.
|
||||||
let (_, _, kind) = this.get_alloc_info(*alloc_id);
|
let (_, _, kind) = this.get_alloc_info(*alloc_id);
|
||||||
if matches!(kind, AllocKind::LiveData) {
|
if matches!(kind, AllocKind::LiveData) {
|
||||||
let alloc_extra = this.get_alloc_extra(*alloc_id).unwrap();
|
let alloc_extra = this.get_alloc_extra(*alloc_id)?; // can still fail for `extern static`
|
||||||
let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap();
|
let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap();
|
||||||
alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?;
|
alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?;
|
||||||
}
|
}
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
//! `Machine` trait.
|
//! `Machine` trait.
|
||||||
|
|
||||||
use std::borrow::Cow;
|
use std::borrow::Cow;
|
||||||
use std::cell::{Cell, RefCell};
|
use std::cell::RefCell;
|
||||||
use std::collections::hash_map::Entry;
|
use std::collections::hash_map::Entry;
|
||||||
use std::fmt;
|
use std::fmt;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
|
@ -336,20 +336,11 @@ pub struct AllocExtra<'tcx> {
|
||||||
/// if this allocation is leakable. The backtrace is not
|
/// if this allocation is leakable. The backtrace is not
|
||||||
/// pruned yet; that should be done before printing it.
|
/// pruned yet; that should be done before printing it.
|
||||||
pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
|
pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
|
||||||
/// An offset inside this allocation that was deemed aligned even for symbolic alignment checks.
|
|
||||||
/// Invariant: the promised alignment will never be less than the native alignment of this allocation.
|
|
||||||
pub symbolic_alignment: Cell<Option<(Size, Align)>>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl VisitProvenance for AllocExtra<'_> {
|
impl VisitProvenance for AllocExtra<'_> {
|
||||||
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
|
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
|
||||||
let AllocExtra {
|
let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;
|
||||||
borrow_tracker,
|
|
||||||
data_race,
|
|
||||||
weak_memory,
|
|
||||||
backtrace: _,
|
|
||||||
symbolic_alignment: _,
|
|
||||||
} = self;
|
|
||||||
|
|
||||||
borrow_tracker.visit_provenance(visit);
|
borrow_tracker.visit_provenance(visit);
|
||||||
data_race.visit_provenance(visit);
|
data_race.visit_provenance(visit);
|
||||||
|
@ -572,6 +563,14 @@ pub struct MiriMachine<'mir, 'tcx> {
|
||||||
/// that is fixed per stack frame; this lets us have sometimes different results for the
|
/// that is fixed per stack frame; this lets us have sometimes different results for the
|
||||||
/// same const while ensuring consistent results within a single call.
|
/// same const while ensuring consistent results within a single call.
|
||||||
const_cache: RefCell<FxHashMap<(mir::Const<'tcx>, usize), OpTy<'tcx, Provenance>>>,
|
const_cache: RefCell<FxHashMap<(mir::Const<'tcx>, usize), OpTy<'tcx, Provenance>>>,
|
||||||
|
|
||||||
|
/// For each allocation, an offset inside that allocation that was deemed aligned even for
|
||||||
|
/// symbolic alignment checks. This cannot be stored in `AllocExtra` since it needs to be
|
||||||
|
/// tracked for vtables and function allocations as well as regular allocations.
|
||||||
|
///
|
||||||
|
/// Invariant: the promised alignment will never be less than the native alignment of the
|
||||||
|
/// allocation.
|
||||||
|
pub(crate) symbolic_alignment: RefCell<FxHashMap<AllocId, (Size, Align)>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
|
impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
|
||||||
|
@ -698,6 +697,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
|
||||||
collect_leak_backtraces: config.collect_leak_backtraces,
|
collect_leak_backtraces: config.collect_leak_backtraces,
|
||||||
allocation_spans: RefCell::new(FxHashMap::default()),
|
allocation_spans: RefCell::new(FxHashMap::default()),
|
||||||
const_cache: RefCell::new(FxHashMap::default()),
|
const_cache: RefCell::new(FxHashMap::default()),
|
||||||
|
symbolic_alignment: RefCell::new(FxHashMap::default()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -810,6 +810,7 @@ impl VisitProvenance for MiriMachine<'_, '_> {
|
||||||
collect_leak_backtraces: _,
|
collect_leak_backtraces: _,
|
||||||
allocation_spans: _,
|
allocation_spans: _,
|
||||||
const_cache: _,
|
const_cache: _,
|
||||||
|
symbolic_alignment: _,
|
||||||
} = self;
|
} = self;
|
||||||
|
|
||||||
threads.visit_provenance(visit);
|
threads.visit_provenance(visit);
|
||||||
|
@ -893,9 +894,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
// Let's see which alignment we have been promised for this allocation.
|
// Let's see which alignment we have been promised for this allocation.
|
||||||
let alloc_info = ecx.get_alloc_extra(alloc_id).unwrap(); // cannot fail since the allocation is live
|
let (promised_offset, promised_align) = ecx
|
||||||
let (promised_offset, promised_align) =
|
.machine
|
||||||
alloc_info.symbolic_alignment.get().unwrap_or((Size::ZERO, alloc_align));
|
.symbolic_alignment
|
||||||
|
.borrow()
|
||||||
|
.get(&alloc_id)
|
||||||
|
.copied()
|
||||||
|
.unwrap_or((Size::ZERO, alloc_align));
|
||||||
if promised_align < align {
|
if promised_align < align {
|
||||||
// Definitely not enough.
|
// Definitely not enough.
|
||||||
Some(Misalignment { has: promised_align, required: align })
|
Some(Misalignment { has: promised_align, required: align })
|
||||||
|
@ -1132,7 +1137,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
|
||||||
data_race: race_alloc,
|
data_race: race_alloc,
|
||||||
weak_memory: buffer_alloc,
|
weak_memory: buffer_alloc,
|
||||||
backtrace,
|
backtrace,
|
||||||
symbolic_alignment: Cell::new(None),
|
|
||||||
},
|
},
|
||||||
|ptr| ecx.global_base_pointer(ptr),
|
|ptr| ecx.global_base_pointer(ptr),
|
||||||
)?;
|
)?;
|
||||||
|
|
|
@ -196,6 +196,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
|
||||||
let this = self.eval_context_mut();
|
let this = self.eval_context_mut();
|
||||||
let allocs = LiveAllocs { ecx: this, collected: allocs };
|
let allocs = LiveAllocs { ecx: this, collected: allocs };
|
||||||
this.machine.allocation_spans.borrow_mut().retain(|id, _| allocs.is_live(*id));
|
this.machine.allocation_spans.borrow_mut().retain(|id, _| allocs.is_live(*id));
|
||||||
|
this.machine.symbolic_alignment.borrow_mut().retain(|id, _| allocs.is_live(*id));
|
||||||
this.machine.intptrcast.borrow_mut().remove_unreachable_allocs(&allocs);
|
this.machine.intptrcast.borrow_mut().remove_unreachable_allocs(&allocs);
|
||||||
if let Some(borrow_tracker) = &this.machine.borrow_tracker {
|
if let Some(borrow_tracker) = &this.machine.borrow_tracker {
|
||||||
borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs);
|
borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs);
|
||||||
|
|
|
@ -585,17 +585,17 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
|
||||||
}
|
}
|
||||||
if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) {
|
if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) {
|
||||||
let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id);
|
let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id);
|
||||||
// Not `get_alloc_extra_mut`, need to handle read-only allocations!
|
|
||||||
let alloc_extra = this.get_alloc_extra(alloc_id)?;
|
|
||||||
// If the newly promised alignment is bigger than the native alignment of this
|
// If the newly promised alignment is bigger than the native alignment of this
|
||||||
// allocation, and bigger than the previously promised alignment, then set it.
|
// allocation, and bigger than the previously promised alignment, then set it.
|
||||||
if align > alloc_align
|
if align > alloc_align
|
||||||
&& !alloc_extra
|
&& !this
|
||||||
|
.machine
|
||||||
.symbolic_alignment
|
.symbolic_alignment
|
||||||
.get()
|
.get_mut()
|
||||||
.is_some_and(|(_, old_align)| align <= old_align)
|
.get(&alloc_id)
|
||||||
|
.is_some_and(|&(_, old_align)| align <= old_align)
|
||||||
{
|
{
|
||||||
alloc_extra.symbolic_alignment.set(Some((offset, align)));
|
this.machine.symbolic_alignment.get_mut().insert(alloc_id, (offset, align));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -0,0 +1,12 @@
|
||||||
|
//@compile-flags: -Zmiri-symbolic-alignment-check
|
||||||
|
|
||||||
|
extern "C" {
|
||||||
|
static _dispatch_queue_attr_concurrent: [u8; 0];
|
||||||
|
}
|
||||||
|
|
||||||
|
static DISPATCH_QUEUE_CONCURRENT: &'static [u8; 0] =
|
||||||
|
unsafe { &_dispatch_queue_attr_concurrent };
|
||||||
|
|
||||||
|
fn main() {
|
||||||
|
let _val = *DISPATCH_QUEUE_CONCURRENT; //~ERROR: is not supported
|
||||||
|
}
|
|
@ -0,0 +1,14 @@
|
||||||
|
error: unsupported operation: `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri
|
||||||
|
--> $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC
|
||||||
|
|
|
||||||
|
LL | let _val = *DISPATCH_QUEUE_CONCURRENT;
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^ `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri
|
||||||
|
|
|
||||||
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
|
||||||
|
= note: BACKTRACE:
|
||||||
|
= note: inside `main` at $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC
|
||||||
|
|
||||||
|
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
|
||||||
|
|
||||||
|
error: aborting due to 1 previous error
|
||||||
|
|
|
@ -1,6 +1,8 @@
|
||||||
//@compile-flags: -Zmiri-symbolic-alignment-check
|
//@compile-flags: -Zmiri-symbolic-alignment-check
|
||||||
#![feature(strict_provenance)]
|
#![feature(strict_provenance)]
|
||||||
|
|
||||||
|
use std::mem;
|
||||||
|
|
||||||
fn test_align_to() {
|
fn test_align_to() {
|
||||||
const N: usize = 4;
|
const N: usize = 4;
|
||||||
let d = Box::new([0u32; N]);
|
let d = Box::new([0u32; N]);
|
||||||
|
@ -68,7 +70,7 @@ fn test_u64_array() {
|
||||||
#[repr(align(8))]
|
#[repr(align(8))]
|
||||||
struct AlignToU64<T>(T);
|
struct AlignToU64<T>(T);
|
||||||
|
|
||||||
const BYTE_LEN: usize = std::mem::size_of::<[u64; 4]>();
|
const BYTE_LEN: usize = mem::size_of::<[u64; 4]>();
|
||||||
type Data = AlignToU64<[u8; BYTE_LEN]>;
|
type Data = AlignToU64<[u8; BYTE_LEN]>;
|
||||||
|
|
||||||
fn example(data: &Data) {
|
fn example(data: &Data) {
|
||||||
|
@ -101,10 +103,29 @@ fn huge_align() {
|
||||||
let _ = std::ptr::invalid::<HugeSize>(SIZE).align_offset(SIZE);
|
let _ = std::ptr::invalid::<HugeSize>(SIZE).align_offset(SIZE);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// This shows that we cannot store the promised alignment info in `AllocExtra`,
|
||||||
|
// since vtables do not have an `AllocExtra`.
|
||||||
|
fn vtable() {
|
||||||
|
#[cfg(target_pointer_width = "64")]
|
||||||
|
type TWOPTR = u128;
|
||||||
|
#[cfg(target_pointer_width = "32")]
|
||||||
|
type TWOPTR = u64;
|
||||||
|
|
||||||
|
let ptr: &dyn Send = &0;
|
||||||
|
let parts: (*const (), *const u8) = unsafe { mem::transmute(ptr) };
|
||||||
|
let vtable = parts.1 ;
|
||||||
|
let offset = vtable.align_offset(mem::align_of::<TWOPTR>());
|
||||||
|
let _vtable_aligned = vtable.wrapping_add(offset) as *const [TWOPTR; 0];
|
||||||
|
// FIXME: we can't actually do the access since vtable pointers act like zero-sized allocations.
|
||||||
|
// Enable the next line once https://github.com/rust-lang/rust/issues/117945 is implemented.
|
||||||
|
//let _place = unsafe { &*vtable_aligned };
|
||||||
|
}
|
||||||
|
|
||||||
fn main() {
|
fn main() {
|
||||||
test_align_to();
|
test_align_to();
|
||||||
test_from_utf8();
|
test_from_utf8();
|
||||||
test_u64_array();
|
test_u64_array();
|
||||||
test_cstr();
|
test_cstr();
|
||||||
huge_align();
|
huge_align();
|
||||||
|
vtable();
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue