From 02897e03cfe6b04d28c0f3197563d1dba3f7658d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 28 Mar 2020 09:50:24 +0100 Subject: [PATCH 1/2] cleanup tcx usage and a few comments --- src/shims/foreign_items.rs | 6 +++--- src/shims/foreign_items/posix.rs | 5 ++--- src/shims/foreign_items/windows.rs | 3 +-- src/shims/intrinsics.rs | 3 +-- src/shims/tls.rs | 7 +++++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 6827b72427..0bed688187 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -124,7 +124,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx }; // Strip linker suffixes (seen on 32-bit macOS). let link_name = link_name.trim_end_matches("$UNIX2003"); - let tcx = &{ this.tcx.tcx }; + let tcx = this.tcx.tcx; // First: functions that diverge. let (dest, ret) = match ret { @@ -133,8 +133,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // The implementation is provided by the function with the `#[panic_handler]` attribute. "panic_impl" => { this.check_panic_supported()?; - let panic_impl_id = this.tcx.lang_items().panic_impl().unwrap(); - let panic_impl_instance = ty::Instance::mono(*this.tcx, panic_impl_id); + let panic_impl_id = tcx.lang_items().panic_impl().unwrap(); + let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id); return Ok(Some(&*this.load_mir(panic_impl_instance.def, None)?)); } | "exit" diff --git a/src/shims/foreign_items/posix.rs b/src/shims/foreign_items/posix.rs index 85e9b88b6e..425fe4b1b4 100644 --- a/src/shims/foreign_items/posix.rs +++ b/src/shims/foreign_items/posix.rs @@ -17,7 +17,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ret: mir::BasicBlock, ) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - let tcx = &{ this.tcx.tcx }; match link_name { // Environment related shims @@ -65,7 +64,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx "write" => { let fd = this.read_scalar(args[0])?.to_i32()?; let buf = this.read_scalar(args[1])?.not_undef()?; - let n = this.read_scalar(args[2])?.to_machine_usize(tcx)?; + let n = this.read_scalar(args[2])?.to_machine_usize(this)?; trace!("Called write({:?}, {:?}, {:?})", fd, buf, n); let result = if fd == 1 || fd == 2 { // stdout/stderr @@ -209,7 +208,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "pthread_getspecific" => { let key = this.force_bits(this.read_scalar(args[0])?.not_undef()?, args[0].layout.size)?; - let ptr = this.machine.tls.load_tls(key, tcx)?; + let ptr = this.machine.tls.load_tls(key, this)?; this.write_scalar(ptr, dest)?; } "pthread_setspecific" => { diff --git a/src/shims/foreign_items/windows.rs b/src/shims/foreign_items/windows.rs index a64ef0f129..9e71ba7d90 100644 --- a/src/shims/foreign_items/windows.rs +++ b/src/shims/foreign_items/windows.rs @@ -13,7 +13,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx _ret: mir::BasicBlock, ) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - let tcx = &{ this.tcx.tcx }; match link_name { // Windows API stubs. @@ -160,7 +159,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "TlsGetValue" => { let key = u128::from(this.read_scalar(args[0])?.to_u32()?); - let ptr = this.machine.tls.load_tls(key, tcx)?; + let ptr = this.machine.tls.load_tls(key, this)?; this.write_scalar(ptr, dest)?; } "TlsSetValue" => { diff --git a/src/shims/intrinsics.rs b/src/shims/intrinsics.rs index 91371e2c9d..84e8cca556 100644 --- a/src/shims/intrinsics.rs +++ b/src/shims/intrinsics.rs @@ -24,13 +24,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if this.emulate_intrinsic(span, instance, args, ret)? { return Ok(()); } - let tcx = &{ this.tcx.tcx }; let substs = instance.substs; // All these intrinsics take raw pointers, so if we access memory directly // (as opposed to through a place), we have to remember to erase any tag // that might still hang around! - let intrinsic_name = &*tcx.item_name(instance.def_id()).as_str(); + let intrinsic_name = &*this.tcx.item_name(instance.def_id()).as_str(); // First handle intrinsics without return place. let (dest, ret) = match ret { diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 6635978cb2..461b5b3eed 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -70,7 +70,7 @@ impl<'tcx> TlsData<'tcx> { } pub fn load_tls( - &mut self, + &self, key: TlsKey, cx: &impl HasDataLayout, ) -> InterpResult<'tcx, Scalar> { @@ -107,7 +107,8 @@ impl<'tcx> TlsData<'tcx> { Ok(()) } - /// Returns a dtor, its argument and its index, if one is supposed to run + /// Returns a dtor, its argument and its index, if one is supposed to run. + /// `key` is the last dtors that was run; we return the *next* one after that. /// /// An optional destructor function may be associated with each key value. /// At thread exit, if a key value has a non-NULL destructor pointer, @@ -191,8 +192,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // step until out of stackframes this.run()?; + // Fetch next dtor after `key`. dtor = match this.machine.tls.fetch_tls_dtor(Some(key)) { dtor @ Some(_) => dtor, + // We ran each dtor once, start over from the beginning. None => this.machine.tls.fetch_tls_dtor(None), }; } From 876bded2e8d5fe0dc3b084c3e9faa2739493d797 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 28 Mar 2020 10:07:23 +0100 Subject: [PATCH 2/2] run Windows TLS dtor function --- src/eval.rs | 7 +++---- src/helpers.rs | 2 +- src/machine.rs | 34 ++++++++++++++++++++++++---------- src/shims/foreign_items.rs | 2 +- src/shims/tls.rs | 27 ++++++++++++++++++++++++++- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 7f07fc1a7d..512b4176df 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -184,10 +184,9 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( /// Returns `Some(return_code)` if program executed completed. /// Returns `None` if an evaluation error occured. pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> Option { - // FIXME: We always ignore leaks on some OSs where we do not - // correctly implement TLS destructors. - let target_os = &tcx.sess.target.target.target_os; - let ignore_leaks = config.ignore_leaks || target_os == "windows"; + // FIXME: on Windows, locks and TLS dtor management allocate and leave that memory in `static`s. + // So we need https://github.com/rust-lang/miri/issues/940 to fix the leaks there. + let ignore_leaks = config.ignore_leaks || tcx.sess.target.target.target_os == "windows"; let (mut ecx, ret_place) = match create_ecx(tcx, main_id, config) { Ok(v) => v, diff --git a/src/helpers.rs b/src/helpers.rs index 3e89fdf6f3..aa327b468b 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -413,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'tcx> { use std::io::ErrorKind::*; let this = self.eval_context_mut(); - let target = &this.tcx.tcx.sess.target.target; + let target = &this.tcx.sess.target.target; let last_error = if target.options.target_family == Some("unix".to_owned()) { this.eval_libc(match e.kind() { ConnectionRefused => "ECONNREFUSED", diff --git a/src/machine.rs b/src/machine.rs index 73e7ce665b..4bf3d0d7f4 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -102,6 +102,20 @@ impl MemoryExtra { } } + fn add_extern_static<'tcx, 'mir>( + this: &mut MiriEvalContext<'mir, 'tcx>, + name: &str, + ptr: Scalar, + ) { + let ptr = ptr.assert_ptr(); + assert_eq!(ptr.offset, Size::ZERO); + this.memory + .extra + .extern_statics + .insert(Symbol::intern(name), ptr.alloc_id) + .unwrap_none(); + } + /// Sets up the "extern statics" for this machine. pub fn init_extern_statics<'tcx, 'mir>( this: &mut MiriEvalContext<'mir, 'tcx>, @@ -113,17 +127,17 @@ impl MemoryExtra { let layout = this.layout_of(this.tcx.types.usize)?; let place = this.allocate(layout, MiriMemoryKind::Machine.into()); this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; - this.memory - .extra - .extern_statics - .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) - .unwrap_none(); + Self::add_extern_static(this, "__cxa_thread_atexit_impl", place.ptr); // "environ" - this.memory - .extra - .extern_statics - .insert(Symbol::intern("environ"), this.machine.env_vars.environ.unwrap().ptr.assert_ptr().alloc_id) - .unwrap_none(); + Self::add_extern_static(this, "environ", this.machine.env_vars.environ.unwrap().ptr); + } + "windows" => { + // "_tls_used" + // This is some obscure hack that is part of the Windows TLS story. It's a `u8`. + let layout = this.layout_of(this.tcx.types.u8)?; + let place = this.allocate(layout, MiriMemoryKind::Machine.into()); + this.write_scalar(Scalar::from_u8(0), place.into())?; + Self::add_extern_static(this, "_tls_used", place.ptr); } _ => {} // No "extern statics" supported on this target } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 0bed688187..ecf24e2f20 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -19,7 +19,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn min_align(&self, size: u64, kind: MiriMemoryKind) -> Align { let this = self.eval_context_ref(); // List taken from `libstd/sys_common/alloc.rs`. - let min_align = match this.tcx.tcx.sess.target.target.arch.as_str() { + let min_align = match this.tcx.sess.target.target.arch.as_str() { "x86" | "arm" | "mips" | "powerpc" | "powerpc64" | "asmjs" | "wasm32" => 8, "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" => 16, arch => bug!("Unsupported target architecture: {}", arch), diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 461b5b3eed..c753689f4c 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -159,6 +159,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx assert!(!this.machine.tls.dtors_running, "running TLS dtors twice"); this.machine.tls.dtors_running = true; + if this.tcx.sess.target.target.target_os == "windows" { + // Windows has a special magic linker section that is run on certain events. + // Instead of searching for that section and supporting arbitrary hooks in there + // (that would be basically https://github.com/rust-lang/miri/issues/450), + // we specifically look up the static in libstd that we know is placed + // in that section. + let thread_callback = this.eval_path_scalar(&["std", "sys", "windows", "thread_local", "p_thread_callback"])?; + let thread_callback = this.memory.get_fn(thread_callback.not_undef()?)?.as_instance()?; + + // The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`. + let reason = this.eval_path_scalar(&["std", "sys", "windows", "c", "DLL_PROCESS_DETACH"])?; + let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into(); + this.call_function( + thread_callback, + &[Scalar::ptr_null(this).into(), reason.into(), Scalar::ptr_null(this).into()], + Some(ret_place), + StackPopCleanup::None { cleanup: true }, + )?; + + // step until out of stackframes + this.run()?; + + // Windows doesn't have other destructors. + return Ok(()); + } + // The macOS global dtor runs "before any TLS slots get freed", so do that first. if let Some((instance, data)) = this.machine.tls.global_dtor { trace!("Running global dtor {:?} on {:?}", instance, data); @@ -199,7 +225,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx None => this.machine.tls.fetch_tls_dtor(None), }; } - // FIXME: On a windows target, call `unsafe extern "system" fn on_tls_callback`. Ok(()) } }