Skip to content

Commit

Permalink
Auto merge of #2721 - a-b-c-1-2-3:fix-pagesize, r=RalfJung
Browse files Browse the repository at this point in the history
Allow configurable and platform-specific page sizes

This fixes #2644 by setting platform-default page sizes along with a command line flag to override size to a specific value (e.g. in the case of aarch64 Linux on M1 silicon). There's still some code cleanup to be done and tests need to be added but I'm opening this for now.
  • Loading branch information
bors committed Dec 11, 2022
2 parents ac7a752 + 22628ff commit 0805372
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 16 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,8 @@ to Miri failing to detect cases of undefined behavior in a program.
* `-Zmiri-track-weak-memory-loads` shows a backtrace when weak memory emulation returns an outdated
value from a load. This can help diagnose problems that disappear under
`-Zmiri-disable-weak-memory-emulation`.
* `-Zmiri-force-page-size=<num>` overrides the default page size for an architecture, in multiples of 1k.
`4` is default for most targets. This value should always be a power of 2 and nonzero.

[function ABI]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier

Expand Down
12 changes: 12 additions & 0 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,18 @@ fn main() {
};

miri_config.num_cpus = num_cpus;
} else if let Some(param) = arg.strip_prefix("-Zmiri-force-page-size=") {
let page_size = match param.parse::<u64>() {
Ok(i) =>
if i.is_power_of_two() {
i * 1024
} else {
show_error!("-Zmiri-force-page-size requires a power of 2: {}", i)
},
Err(err) => show_error!("-Zmiri-force-page-size requires a `u64`: {}", err),
};

miri_config.page_size = Some(page_size);
} else {
// Forward to rustc.
rustc_args.push(arg);
Expand Down
3 changes: 3 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ pub struct MiriConfig {
pub gc_interval: u32,
/// The number of CPUs to be reported by miri.
pub num_cpus: u32,
/// Requires Miri to emulate pages of a certain size
pub page_size: Option<u64>,
}

impl Default for MiriConfig {
Expand Down Expand Up @@ -176,6 +178,7 @@ impl Default for MiriConfig {
external_so_file: None,
gc_interval: 10_000,
num_cpus: 1,
page_size: None,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/intptrcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ impl VisitTags for GlobalStateInner {
}

impl GlobalStateInner {
pub fn new(config: &MiriConfig) -> Self {
pub fn new(config: &MiriConfig, stack_addr: u64) -> Self {
GlobalStateInner {
int_to_ptr_map: Vec::default(),
base_addr: FxHashMap::default(),
exposed: FxHashSet::default(),
next_base_addr: STACK_ADDR,
next_base_addr: stack_addr,
provenance_mode: config.provenance_mode,
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub use crate::helpers::EvalContextExt as _;
pub use crate::intptrcast::ProvenanceMode;
pub use crate::machine::{
AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
PrimitiveLayouts, Provenance, ProvenanceExtra, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
PrimitiveLayouts, Provenance, ProvenanceExtra,
};
pub use crate::mono_hash_map::MonoHashMap;
pub use crate::operator::EvalContextExt as _;
Expand Down
37 changes: 31 additions & 6 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ use crate::{
*,
};

// Some global facts about the emulated machine.
pub const PAGE_SIZE: u64 = 4 * 1024; // FIXME: adjust to target architecture
pub const STACK_ADDR: u64 = 32 * PAGE_SIZE; // not really about the "stack", but where we start assigning integer addresses to allocations
pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever

/// Extra data stored with each stack frame
pub struct FrameExtra<'tcx> {
/// Extra data for Stacked Borrows.
Expand Down Expand Up @@ -469,6 +464,10 @@ pub struct MiriMachine<'mir, 'tcx> {
pub(crate) since_gc: u32,
/// The number of CPUs to be reported by miri.
pub(crate) num_cpus: u32,
/// Determines Miri's page size and associated values
pub(crate) page_size: u64,
pub(crate) stack_addr: u64,
pub(crate) stack_size: u64,
}

impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
Expand All @@ -482,11 +481,31 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0));
let borrow_tracker = config.borrow_tracker.map(|bt| bt.instanciate_global_state(config));
let data_race = config.data_race_detector.then(|| data_race::GlobalState::new(config));
let page_size = if let Some(page_size) = config.page_size {
page_size
} else {
let target = &layout_cx.tcx.sess.target;
match target.arch.as_ref() {
"wasm32" | "wasm64" => 64 * 1024, // https://webassembly.github.io/spec/core/exec/runtime.html#memory-instances
"aarch64" =>
if target.options.vendor.as_ref() == "apple" {
// No "definitive" source, but see:
// https://www.wwdcnotes.com/notes/wwdc20/10214/
// https://github.com/ziglang/zig/issues/11308 etc.
16 * 1024
} else {
4 * 1024
},
_ => 4 * 1024,
}
};
let stack_addr = page_size * 32;
let stack_size = page_size * 16;
MiriMachine {
tcx: layout_cx.tcx,
borrow_tracker,
data_race,
intptrcast: RefCell::new(intptrcast::GlobalStateInner::new(config)),
intptrcast: RefCell::new(intptrcast::GlobalStateInner::new(config, stack_addr)),
// `env_vars` depends on a full interpreter so we cannot properly initialize it yet.
env_vars: EnvVars::default(),
main_fn_ret_place: None,
Expand Down Expand Up @@ -548,6 +567,9 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
gc_interval: config.gc_interval,
since_gc: 0,
num_cpus: config.num_cpus,
page_size,
stack_addr,
stack_size,
}
}

Expand Down Expand Up @@ -692,6 +714,9 @@ impl VisitTags for MiriMachine<'_, '_> {
gc_interval: _,
since_gc: _,
num_cpus: _,
page_size: _,
stack_addr: _,
stack_size: _,
} = self;

threads.visit_tags(visit);
Expand Down
8 changes: 4 additions & 4 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// FIXME: Which of these are POSIX, and which are GNU/Linux?
// At least the names seem to all also exist on macOS.
let sysconfs: &[(&str, fn(&MiriInterpCx<'_, '_>) -> Scalar<Provenance>)] = &[
("_SC_PAGESIZE", |this| Scalar::from_int(PAGE_SIZE, this.pointer_size())),
("_SC_PAGESIZE", |this| Scalar::from_int(this.machine.page_size, this.pointer_size())),
("_SC_NPROCESSORS_CONF", |this| Scalar::from_int(this.machine.num_cpus, this.pointer_size())),
("_SC_NPROCESSORS_ONLN", |this| Scalar::from_int(this.machine.num_cpus, this.pointer_size())),
// 512 seems to be a reasonable default. The value is not critical, in
Expand Down Expand Up @@ -496,7 +496,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let [_attr, guard_size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let guard_size = this.deref_operand(guard_size)?;
let guard_size_layout = this.libc_ty_layout("size_t")?;
this.write_scalar(Scalar::from_uint(crate::PAGE_SIZE, guard_size_layout.size), &guard_size.into())?;
this.write_scalar(Scalar::from_uint(this.machine.page_size, guard_size_layout.size), &guard_size.into())?;

// Return success (`0`).
this.write_null(dest)?;
Expand Down Expand Up @@ -525,11 +525,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let size_place = this.deref_operand(size_place)?;

this.write_scalar(
Scalar::from_uint(STACK_ADDR, this.pointer_size()),
Scalar::from_uint(this.machine.stack_addr, this.pointer_size()),
&addr_place.into(),
)?;
this.write_scalar(
Scalar::from_uint(STACK_SIZE, this.pointer_size()),
Scalar::from_uint(this.machine.stack_size, this.pointer_size()),
&size_place.into(),
)?;

Expand Down
4 changes: 2 additions & 2 deletions src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
"pthread_get_stackaddr_np" => {
let [thread] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
this.read_scalar(thread)?.to_machine_usize(this)?;
let stack_addr = Scalar::from_uint(STACK_ADDR, this.pointer_size());
let stack_addr = Scalar::from_uint(this.machine.stack_addr, this.pointer_size());
this.write_scalar(stack_addr, dest)?;
}
"pthread_get_stacksize_np" => {
let [thread] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
this.read_scalar(thread)?.to_machine_usize(this)?;
let stack_size = Scalar::from_uint(STACK_SIZE, this.pointer_size());
let stack_size = Scalar::from_uint(this.machine.stack_size, this.pointer_size());
this.write_scalar(stack_size, dest)?;
}

Expand Down
2 changes: 1 addition & 1 deletion src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// Set page size.
let page_size = system_info.offset(field_offsets[2], dword_layout, &this.tcx)?;
this.write_scalar(
Scalar::from_int(PAGE_SIZE, dword_layout.size),
Scalar::from_int(this.machine.page_size, dword_layout.size),
&page_size.into(),
)?;
// Set number of processors.
Expand Down
13 changes: 13 additions & 0 deletions tests/pass-dep/page_size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,17 @@ fn main() {

// In particular, this checks that it is not 0.
assert!(page_size.is_power_of_two(), "page size not a power of two: {}", page_size);
// Most architectures have 4k pages by default
#[cfg(not(any(
target_arch = "wasm32",
target_arch = "wasm64",
all(target_arch = "aarch64", target_vendor = "apple")
)))]
assert!(page_size == 4 * 1024, "non-4k default page size: {}", page_size);
// ... except aarch64-apple with 16k
#[cfg(all(target_arch = "aarch64", target_vendor = "apple"))]
assert!(page_size == 16 * 1024, "aarch64 apple reports non-16k page size: {}", page_size);
// ... and wasm with 64k
#[cfg(any(target_arch = "wasm32", target_arch = "wasm64"))]
assert!(page_size == 64 * 1024, "wasm reports non-64k page size: {}", page_size);
}
7 changes: 7 additions & 0 deletions tests/pass-dep/page_size_override.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//@compile-flags: -Zmiri-force-page-size=8

fn main() {
let page_size = page_size::get();

assert!(page_size == 8 * 1024, "8k page size override not respected: {}", page_size);
}

0 comments on commit 0805372

Please sign in to comment.