From 879000b133aed8cc1893c84eb5319b491a4756d9 Mon Sep 17 00:00:00 2001 From: hyd-dev Date: Thu, 3 Jun 2021 17:58:24 +0800 Subject: [PATCH 1/5] Detect `std` by checking if the crate defines `#[lang = "start"]` rather than string comparison --- src/helpers.rs | 6 +++ src/shims/posix/foreign_items.rs | 10 ++--- src/shims/posix/linux/foreign_items.rs | 4 +- src/shims/posix/macos/foreign_items.rs | 2 +- src/shims/windows/foreign_items.rs | 22 +++-------- .../unsupported_get_process_heap.rs | 12 ++++++ tests/compile-fail/unsupported_signal.rs | 12 ++++++ tests/run-pass/extern_crate_std_in_main.rs | 5 +++ tests/run-pass/rename_std.rs | 5 +++ tests/run-pass/std_only_foreign_function.rs | 39 +++++++++++++++++++ 10 files changed, 92 insertions(+), 25 deletions(-) create mode 100644 tests/compile-fail/unsupported_get_process_heap.rs create mode 100644 tests/compile-fail/unsupported_signal.rs create mode 100644 tests/run-pass/extern_crate_std_in_main.rs create mode 100644 tests/run-pass/rename_std.rs create mode 100644 tests/run-pass/std_only_foreign_function.rs diff --git a/src/helpers.rs b/src/helpers.rs index b71f830b2b..7f99aa1997 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -628,6 +628,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } Ok(()) } + + fn in_std(&self) -> bool { + let this = self.eval_context_ref(); + this.tcx.def_path(this.frame().instance.def_id()).krate + == this.tcx.def_path(this.tcx.lang_items().start_fn().unwrap()).krate + } } /// Check that the number of args is what we expect. diff --git a/src/shims/posix/foreign_items.rs b/src/shims/posix/foreign_items.rs index a756256aab..bbd46af52a 100644 --- a/src/shims/posix/foreign_items.rs +++ b/src/shims/posix/foreign_items.rs @@ -475,7 +475,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. "pthread_attr_getguardsize" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[ref _attr, ref guard_size] = check_arg_count(args)?; let guard_size = this.deref_operand(guard_size)?; @@ -488,13 +488,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "pthread_attr_init" | "pthread_attr_destroy" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_] = check_arg_count(args)?; this.write_null(dest)?; } | "pthread_attr_setstacksize" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _] = check_arg_count(args)?; this.write_null(dest)?; @@ -502,14 +502,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "signal" | "sigaltstack" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _] = check_arg_count(args)?; this.write_null(dest)?; } | "sigaction" | "mprotect" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _, _] = check_arg_count(args)?; this.write_null(dest)?; diff --git a/src/shims/posix/linux/foreign_items.rs b/src/shims/posix/linux/foreign_items.rs index 598d89fe1f..9b0576662f 100644 --- a/src/shims/posix/linux/foreign_items.rs +++ b/src/shims/posix/linux/foreign_items.rs @@ -207,9 +207,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "pthread_getattr_np" - if this.frame().instance.to_string().starts_with("std::sys::unix::") => - { + "pthread_getattr_np" if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[ref _thread, ref _attr] = check_arg_count(args)?; this.write_null(dest)?; diff --git a/src/shims/posix/macos/foreign_items.rs b/src/shims/posix/macos/foreign_items.rs index d54e1bfbe8..1467a95a0d 100644 --- a/src/shims/posix/macos/foreign_items.rs +++ b/src/shims/posix/macos/foreign_items.rs @@ -149,7 +149,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "mmap" if this.frame().instance.to_string().starts_with("std::sys::unix::") => { + "mmap" if this.in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; // This is a horrible hack, but since the guard page mechanism calls mmap and expects a particular return value, we just give it that value. let &[ref addr, _, _, _, _, _] = check_arg_count(args)?; diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index 3c7451352d..fd845cde31 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -348,35 +348,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "GetProcessHeap" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => - { + "GetProcessHeap" if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; let &[] = check_arg_count(args)?; // Just fake a HANDLE this.write_scalar(Scalar::from_machine_isize(1, this), dest)?; } - "SetConsoleTextAttribute" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => - { + "SetConsoleTextAttribute" if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _hConsoleOutput, ref _wAttribute] = check_arg_count(args)?; // Pretend these does not exist / nothing happened, by returning zero. this.write_null(dest)?; } - "AddVectoredExceptionHandler" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => - { + "AddVectoredExceptionHandler" if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _First, ref _Handler] = check_arg_count(args)?; // Any non zero value works for the stdlib. This is just used for stack overflows anyway. this.write_scalar(Scalar::from_machine_usize(1, this), dest)?; } - "SetThreadStackGuarantee" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => - { + "SetThreadStackGuarantee" if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[_StackSizeInBytes] = check_arg_count(args)?; @@ -387,7 +379,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "EnterCriticalSection" | "LeaveCriticalSection" | "DeleteCriticalSection" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => + if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] @@ -401,9 +393,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // (Windows locks are reentrant, and we have only 1 thread, // so not doing any futher checks here is at least not incorrect.) } - "TryEnterCriticalSection" - if this.frame().instance.to_string().starts_with("std::sys::windows::") => - { + "TryEnterCriticalSection" if this.in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _lpCriticalSection] = check_arg_count(args)?; diff --git a/tests/compile-fail/unsupported_get_process_heap.rs b/tests/compile-fail/unsupported_get_process_heap.rs new file mode 100644 index 0000000000..4fd5f4b7d8 --- /dev/null +++ b/tests/compile-fail/unsupported_get_process_heap.rs @@ -0,0 +1,12 @@ +//! `GetProcessHeap()` is special on Windows that it's only supported within libstd. +//! (On Linux and macOS, it's just always unsupported.) + +fn main() { + extern "system" { + fn GetProcessHeap() -> *mut std::ffi::c_void; + } + unsafe { + GetProcessHeap(); + //~^ ERROR unsupported operation: can't call foreign function: GetProcessHeap + } +} diff --git a/tests/compile-fail/unsupported_signal.rs b/tests/compile-fail/unsupported_signal.rs new file mode 100644 index 0000000000..7931747e0a --- /dev/null +++ b/tests/compile-fail/unsupported_signal.rs @@ -0,0 +1,12 @@ +//! `signal()` is special on Linux and macOS that it's only supported within libstd. +// ignore-windows: No libc on Windows +#![feature(rustc_private)] + +extern crate libc; + +fn main() { + unsafe { + libc::signal(libc::SIGPIPE, libc::SIG_IGN); + //~^ ERROR unsupported operation: can't call foreign function: signal + } +} diff --git a/tests/run-pass/extern_crate_std_in_main.rs b/tests/run-pass/extern_crate_std_in_main.rs new file mode 100644 index 0000000000..8f71e61337 --- /dev/null +++ b/tests/run-pass/extern_crate_std_in_main.rs @@ -0,0 +1,5 @@ +#![no_std] + +fn main() { + extern crate std; +} diff --git a/tests/run-pass/rename_std.rs b/tests/run-pass/rename_std.rs new file mode 100644 index 0000000000..7e82e53e6b --- /dev/null +++ b/tests/run-pass/rename_std.rs @@ -0,0 +1,5 @@ +#![no_std] + +extern crate std as foo; + +fn main() {} diff --git a/tests/run-pass/std_only_foreign_function.rs b/tests/run-pass/std_only_foreign_function.rs new file mode 100644 index 0000000000..959103ee5c --- /dev/null +++ b/tests/run-pass/std_only_foreign_function.rs @@ -0,0 +1,39 @@ +//! Make sure we can call foreign functions that are only allowed within libstd if we are "libstd" +//! (defining the `start` lang item). +#![feature(lang_items, rustc_private, core_intrinsics)] +#![no_std] + +use core::{intrinsics, panic::PanicInfo}; + +#[lang = "eh_personality"] +fn rust_eh_personality() {} + +#[panic_handler] +fn panic_handler(_: &PanicInfo<'_>) -> ! { + intrinsics::abort() +} + +#[lang = "start"] +fn start(main: fn(), _argc: isize, _argv: *const *const u8) -> isize { + main(); + 0 +} + +fn main() { + #[cfg(unix)] + unsafe { + extern crate libc; + assert_eq!(libc::signal(libc::SIGPIPE, libc::SIG_IGN), 0); + } + #[cfg(windows)] + unsafe { + extern "system" { + fn GetProcessHeap() -> *mut core::ffi::c_void; + fn ExitProcess(code: u32) -> !; + } + assert_eq!(GetProcessHeap() as usize, 1); + // Early exit to avoid the requirement of + // `std::sys::windows::thread_local_key::p_thread_callback`. + ExitProcess(0); + } +} From 545101040df050dcc412d714640c95f417194600 Mon Sep 17 00:00:00 2001 From: hyd-dev Date: Thu, 3 Jun 2021 21:26:11 +0800 Subject: [PATCH 2/5] Don't `unwrap()` in `in_std()` --- src/helpers.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 7f99aa1997..a9cafd2a9e 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -631,8 +631,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn in_std(&self) -> bool { let this = self.eval_context_ref(); - this.tcx.def_path(this.frame().instance.def_id()).krate - == this.tcx.def_path(this.tcx.lang_items().start_fn().unwrap()).krate + this.tcx.lang_items().start_fn().map_or(false, |start_fn| { + this.tcx.def_path(this.frame().instance.def_id()).krate + == this.tcx.def_path(start_fn).krate + }) } } From 3871c493b24b00c2b3d11af5aef8bb0045dd7469 Mon Sep 17 00:00:00 2001 From: hyd-dev Date: Sun, 6 Jun 2021 11:21:20 +0800 Subject: [PATCH 3/5] `in_std` -> `frame_in_std` --- src/helpers.rs | 2 +- src/shims/posix/foreign_items.rs | 10 +++++----- src/shims/posix/linux/foreign_items.rs | 2 +- src/shims/posix/macos/foreign_items.rs | 2 +- src/shims/windows/foreign_items.rs | 12 ++++++------ 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index a9cafd2a9e..5217634205 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -629,7 +629,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } - fn in_std(&self) -> bool { + fn frame_in_std(&self) -> bool { let this = self.eval_context_ref(); this.tcx.lang_items().start_fn().map_or(false, |start_fn| { this.tcx.def_path(this.frame().instance.def_id()).krate diff --git a/src/shims/posix/foreign_items.rs b/src/shims/posix/foreign_items.rs index bbd46af52a..2ecea4d9f4 100644 --- a/src/shims/posix/foreign_items.rs +++ b/src/shims/posix/foreign_items.rs @@ -475,7 +475,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. "pthread_attr_getguardsize" - if this.in_std() => { + if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[ref _attr, ref guard_size] = check_arg_count(args)?; let guard_size = this.deref_operand(guard_size)?; @@ -488,13 +488,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "pthread_attr_init" | "pthread_attr_destroy" - if this.in_std() => { + if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_] = check_arg_count(args)?; this.write_null(dest)?; } | "pthread_attr_setstacksize" - if this.in_std() => { + if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _] = check_arg_count(args)?; this.write_null(dest)?; @@ -502,14 +502,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "signal" | "sigaltstack" - if this.in_std() => { + if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _] = check_arg_count(args)?; this.write_null(dest)?; } | "sigaction" | "mprotect" - if this.in_std() => { + if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[_, _, _] = check_arg_count(args)?; this.write_null(dest)?; diff --git a/src/shims/posix/linux/foreign_items.rs b/src/shims/posix/linux/foreign_items.rs index 9b0576662f..9017dc368b 100644 --- a/src/shims/posix/linux/foreign_items.rs +++ b/src/shims/posix/linux/foreign_items.rs @@ -207,7 +207,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "pthread_getattr_np" if this.in_std() => { + "pthread_getattr_np" if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; let &[ref _thread, ref _attr] = check_arg_count(args)?; this.write_null(dest)?; diff --git a/src/shims/posix/macos/foreign_items.rs b/src/shims/posix/macos/foreign_items.rs index 1467a95a0d..313d38c80b 100644 --- a/src/shims/posix/macos/foreign_items.rs +++ b/src/shims/posix/macos/foreign_items.rs @@ -149,7 +149,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "mmap" if this.in_std() => { + "mmap" if this.frame_in_std() => { this.check_abi(abi, Abi::C { unwind: false })?; // This is a horrible hack, but since the guard page mechanism calls mmap and expects a particular return value, we just give it that value. let &[ref addr, _, _, _, _, _] = check_arg_count(args)?; diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index fd845cde31..76657a5355 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -348,27 +348,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. - "GetProcessHeap" if this.in_std() => { + "GetProcessHeap" if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; let &[] = check_arg_count(args)?; // Just fake a HANDLE this.write_scalar(Scalar::from_machine_isize(1, this), dest)?; } - "SetConsoleTextAttribute" if this.in_std() => { + "SetConsoleTextAttribute" if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _hConsoleOutput, ref _wAttribute] = check_arg_count(args)?; // Pretend these does not exist / nothing happened, by returning zero. this.write_null(dest)?; } - "AddVectoredExceptionHandler" if this.in_std() => { + "AddVectoredExceptionHandler" if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _First, ref _Handler] = check_arg_count(args)?; // Any non zero value works for the stdlib. This is just used for stack overflows anyway. this.write_scalar(Scalar::from_machine_usize(1, this), dest)?; } - "SetThreadStackGuarantee" if this.in_std() => { + "SetThreadStackGuarantee" if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[_StackSizeInBytes] = check_arg_count(args)?; @@ -379,7 +379,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx | "EnterCriticalSection" | "LeaveCriticalSection" | "DeleteCriticalSection" - if this.in_std() => + if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] @@ -393,7 +393,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // (Windows locks are reentrant, and we have only 1 thread, // so not doing any futher checks here is at least not incorrect.) } - "TryEnterCriticalSection" if this.in_std() => { + "TryEnterCriticalSection" if this.frame_in_std() => { this.check_abi(abi, Abi::System { unwind: false })?; #[allow(non_snake_case)] let &[ref _lpCriticalSection] = check_arg_count(args)?; From d7aff960538bf3cadd66556ecedc87fa685492d5 Mon Sep 17 00:00:00 2001 From: hyd-dev Date: Sun, 6 Jun 2021 11:22:25 +0800 Subject: [PATCH 4/5] Remove 2 tests --- .../unsupported_get_process_heap.rs | 12 ------ tests/run-pass/std_only_foreign_function.rs | 39 ------------------- 2 files changed, 51 deletions(-) delete mode 100644 tests/compile-fail/unsupported_get_process_heap.rs delete mode 100644 tests/run-pass/std_only_foreign_function.rs diff --git a/tests/compile-fail/unsupported_get_process_heap.rs b/tests/compile-fail/unsupported_get_process_heap.rs deleted file mode 100644 index 4fd5f4b7d8..0000000000 --- a/tests/compile-fail/unsupported_get_process_heap.rs +++ /dev/null @@ -1,12 +0,0 @@ -//! `GetProcessHeap()` is special on Windows that it's only supported within libstd. -//! (On Linux and macOS, it's just always unsupported.) - -fn main() { - extern "system" { - fn GetProcessHeap() -> *mut std::ffi::c_void; - } - unsafe { - GetProcessHeap(); - //~^ ERROR unsupported operation: can't call foreign function: GetProcessHeap - } -} diff --git a/tests/run-pass/std_only_foreign_function.rs b/tests/run-pass/std_only_foreign_function.rs deleted file mode 100644 index 959103ee5c..0000000000 --- a/tests/run-pass/std_only_foreign_function.rs +++ /dev/null @@ -1,39 +0,0 @@ -//! Make sure we can call foreign functions that are only allowed within libstd if we are "libstd" -//! (defining the `start` lang item). -#![feature(lang_items, rustc_private, core_intrinsics)] -#![no_std] - -use core::{intrinsics, panic::PanicInfo}; - -#[lang = "eh_personality"] -fn rust_eh_personality() {} - -#[panic_handler] -fn panic_handler(_: &PanicInfo<'_>) -> ! { - intrinsics::abort() -} - -#[lang = "start"] -fn start(main: fn(), _argc: isize, _argv: *const *const u8) -> isize { - main(); - 0 -} - -fn main() { - #[cfg(unix)] - unsafe { - extern crate libc; - assert_eq!(libc::signal(libc::SIGPIPE, libc::SIG_IGN), 0); - } - #[cfg(windows)] - unsafe { - extern "system" { - fn GetProcessHeap() -> *mut core::ffi::c_void; - fn ExitProcess(code: u32) -> !; - } - assert_eq!(GetProcessHeap() as usize, 1); - // Early exit to avoid the requirement of - // `std::sys::windows::thread_local_key::p_thread_callback`. - ExitProcess(0); - } -} From 0ece55d748a04aba9c4950937de226c259630719 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Jun 2021 10:33:46 +0200 Subject: [PATCH 5/5] expand comment --- tests/compile-fail/unsupported_signal.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/compile-fail/unsupported_signal.rs b/tests/compile-fail/unsupported_signal.rs index 7931747e0a..3e76d1c3f3 100644 --- a/tests/compile-fail/unsupported_signal.rs +++ b/tests/compile-fail/unsupported_signal.rs @@ -1,4 +1,5 @@ //! `signal()` is special on Linux and macOS that it's only supported within libstd. +//! The implementation is not complete enough to permit user code to call it. // ignore-windows: No libc on Windows #![feature(rustc_private)]