From 9aab517d6310223ac5a89c640723a64b695d49d2 Mon Sep 17 00:00:00 2001 From: Andrew Zhogin Date: Tue, 24 Sep 2024 16:35:00 +0700 Subject: [PATCH 1/5] rust_for_linux: -Zreg-struct-return commandline flag for X86 (#116973) --- compiler/rustc_codegen_gcc/src/context.rs | 5 +- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_middle/src/ty/layout.rs | 5 +- compiler/rustc_session/messages.ftl | 1 + compiler/rustc_session/src/errors.rs | 4 + compiler/rustc_session/src/options.rs | 3 + compiler/rustc_session/src/session.rs | 5 + compiler/rustc_target/src/callconv/mod.rs | 4 +- compiler/rustc_target/src/callconv/x86.rs | 3 +- compiler/rustc_target/src/spec/mod.rs | 2 + .../src/compiler-flags/reg-struct-return.md | 15 ++ tests/codegen/reg-struct-return.rs | 206 ++++++++++++++++++ .../requires-x86.aarch64.stderr | 4 + .../reg-struct-return/requires-x86.rs | 21 ++ .../requires-x86.x86_64.stderr | 4 + 15 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/reg-struct-return.md create mode 100644 tests/codegen/reg-struct-return.rs create mode 100644 tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.aarch64.stderr create mode 100644 tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs create mode 100644 tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.x86_64.stderr diff --git a/compiler/rustc_codegen_gcc/src/context.rs b/compiler/rustc_codegen_gcc/src/context.rs index 3846d0255377d..f67dcf0cb116d 100644 --- a/compiler/rustc_codegen_gcc/src/context.rs +++ b/compiler/rustc_codegen_gcc/src/context.rs @@ -544,7 +544,10 @@ impl<'gcc, 'tcx> HasWasmCAbiOpt for CodegenCx<'gcc, 'tcx> { impl<'gcc, 'tcx> HasX86AbiOpt for CodegenCx<'gcc, 'tcx> { fn x86_abi_opt(&self) -> X86Abi { - X86Abi { regparm: self.tcx.sess.opts.unstable_opts.regparm } + X86Abi { + regparm: self.tcx.sess.opts.unstable_opts.regparm, + reg_struct_return: self.tcx.sess.opts.unstable_opts.reg_struct_return, + } } } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 6beae14100d9a..3e74a014093fa 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -831,6 +831,7 @@ fn test_unstable_options_tracking_hash() { tracked!(precise_enum_drop_elaboration, false); tracked!(profile_sample_use, Some(PathBuf::from("abc"))); tracked!(profiler_runtime, "abc".to_string()); + tracked!(reg_struct_return, true); tracked!(regparm, Some(3)); tracked!(relax_elf_relocations, Some(true)); tracked!(remap_cwd_prefix, Some(PathBuf::from("abc"))); diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 01ad76aedc375..07573a7926003 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -552,7 +552,10 @@ impl<'tcx> HasWasmCAbiOpt for TyCtxt<'tcx> { impl<'tcx> HasX86AbiOpt for TyCtxt<'tcx> { fn x86_abi_opt(&self) -> X86Abi { - X86Abi { regparm: self.sess.opts.unstable_opts.regparm } + X86Abi { + regparm: self.sess.opts.unstable_opts.regparm, + reg_struct_return: self.sess.opts.unstable_opts.reg_struct_return, + } } } diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index 8fd87893a98c2..eb14b78a0030b 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -135,5 +135,6 @@ session_unsupported_crate_type_for_target = session_unsupported_dwarf_version = requested DWARF version {$dwarf_version} is greater than 5 +session_unsupported_reg_struct_return_arch = `-Zreg-struct-return` is only supported on x86 session_unsupported_regparm = `-Zregparm={$regparm}` is unsupported (valid values 0-3) session_unsupported_regparm_arch = `-Zregparm=N` is only supported on x86 diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index 736a5ce07049a..6c26a78148719 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -489,6 +489,10 @@ pub(crate) struct UnsupportedRegparm { #[diag(session_unsupported_regparm_arch)] pub(crate) struct UnsupportedRegparmArch; +#[derive(Diagnostic)] +#[diag(session_unsupported_reg_struct_return_arch)] +pub(crate) struct UnsupportedRegStructReturnArch; + #[derive(Diagnostic)] #[diag(session_failed_to_create_profiler)] pub(crate) struct FailedToCreateProfiler { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index a2d75917c8266..ecff392b6774d 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1985,6 +1985,9 @@ options! { "enable queries of the dependency graph for regression testing (default: no)"), randomize_layout: bool = (false, parse_bool, [TRACKED], "randomize the layout of types (default: no)"), + reg_struct_return: bool = (false, parse_bool, [TRACKED], + "On x86-32 targets, it overrides the default ABI to return small structs in registers. + It is UNSOUND to link together crates that use different values for this flag!"), regparm: Option = (None, parse_opt_number, [TRACKED], "On x86-32 targets, setting this to N causes the compiler to pass N arguments \ in registers EAX, EDX, and ECX instead of on the stack for\ diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index f585410adb97d..cc5569836b248 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1305,6 +1305,11 @@ fn validate_commandline_args_with_session_available(sess: &Session) { sess.dcx().emit_err(errors::UnsupportedRegparmArch); } } + if sess.opts.unstable_opts.reg_struct_return { + if sess.target.arch != "x86" { + sess.dcx().emit_err(errors::UnsupportedRegStructReturnArch); + } + } // The code model check applies to `thunk` and `thunk-extern`, but not `thunk-inline`, so it is // kept as a `match` to force a change if new ones are added, even if we currently only support diff --git a/compiler/rustc_target/src/callconv/mod.rs b/compiler/rustc_target/src/callconv/mod.rs index fb0fe4029348e..746e81738076f 100644 --- a/compiler/rustc_target/src/callconv/mod.rs +++ b/compiler/rustc_target/src/callconv/mod.rs @@ -661,7 +661,9 @@ impl<'a, Ty> FnAbi<'a, Ty> { } _ => (x86::Flavor::General, None), }; - x86::compute_abi_info(cx, self, x86::X86Options { flavor, regparm }); + let reg_struct_return = cx.x86_abi_opt().reg_struct_return; + let opts = x86::X86Options { flavor, regparm, reg_struct_return }; + x86::compute_abi_info(cx, self, opts); } "x86_64" => match abi { spec::abi::Abi::SysV64 { .. } => x86_64::compute_abi_info(cx, self), diff --git a/compiler/rustc_target/src/callconv/x86.rs b/compiler/rustc_target/src/callconv/x86.rs index a5af975d4d24d..cd8465c09ca98 100644 --- a/compiler/rustc_target/src/callconv/x86.rs +++ b/compiler/rustc_target/src/callconv/x86.rs @@ -14,6 +14,7 @@ pub(crate) enum Flavor { pub(crate) struct X86Options { pub flavor: Flavor, pub regparm: Option, + pub reg_struct_return: bool, } pub(crate) fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, opts: X86Options) @@ -31,7 +32,7 @@ where // https://www.angelcode.com/dev/callconv/callconv.html // Clang's ABI handling is in lib/CodeGen/TargetInfo.cpp let t = cx.target_spec(); - if t.abi_return_struct_as_int { + if t.abi_return_struct_as_int || opts.reg_struct_return { // According to Clang, everyone but MSVC returns single-element // float aggregates directly in a floating-point register. if !t.is_like_msvc && fn_abi.ret.layout.is_single_fp_element(cx) { diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index fead20ec7d1f9..b2561cfb7babe 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -2108,6 +2108,8 @@ pub struct X86Abi { /// On x86-32 targets, the regparm N causes the compiler to pass arguments /// in registers EAX, EDX, and ECX instead of on the stack. pub regparm: Option, + /// Override the default ABI to return small structs in registers + pub reg_struct_return: bool, } pub trait HasX86AbiOpt { diff --git a/src/doc/unstable-book/src/compiler-flags/reg-struct-return.md b/src/doc/unstable-book/src/compiler-flags/reg-struct-return.md new file mode 100644 index 0000000000000..35b782f38dc4d --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/reg-struct-return.md @@ -0,0 +1,15 @@ +# `reg-struct-return` + +The tracking issue for this feature is: https://github.com/rust-lang/rust/issues/116973. + +------------------------ + +Option -Zreg-struct-return causes the compiler to return small structs in registers +instead of on the stack for extern "C"-like functions. +It is UNSOUND to link together crates that use different values for this flag. +It is only supported on `x86`. + +It is equivalent to [Clang]'s and [GCC]'s `-freg-struct-return`. + +[Clang]: https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-freg-struct-return +[GCC]: https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-freg-struct-return diff --git a/tests/codegen/reg-struct-return.rs b/tests/codegen/reg-struct-return.rs new file mode 100644 index 0000000000000..73816745ea86d --- /dev/null +++ b/tests/codegen/reg-struct-return.rs @@ -0,0 +1,206 @@ +// Checks how `reg-struct-return` flag works with different calling conventions: +// Return struct with 8/16/32/64 bit size will be converted into i8/i16/i32/i64 +// (like abi_return_struct_as_int target spec). +// x86 only. + +//@ revisions: ENABLED DISABLED +//@ add-core-stubs +//@ compile-flags: --target i686-unknown-linux-gnu -O -C no-prepopulate-passes +//@ [ENABLED] compile-flags: -Zreg-struct-return +//@ needs-llvm-components: x86 + +#![crate_type = "lib"] +#![no_std] +#![no_core] +#![feature(no_core, lang_items)] + +extern crate minicore; +use minicore::*; + +#[repr(C)] +pub struct Foo { + x: u32, + y: u32, +} + +#[repr(C)] +pub struct Foo1 { + x: u32, +} + +#[repr(C)] +pub struct Foo2 { + x: bool, + y: bool, + z: i16, +} + +#[repr(C)] +pub struct Foo3 { + x: i16, + y: bool, + z: bool, +} + +#[repr(C)] +pub struct Foo4 { + x: char, + y: bool, + z: u8, +} + +#[repr(C)] +pub struct Foo5 { + x: u32, + y: u16, + z: u8, + a: bool, +} + +#[repr(C)] +pub struct FooOversize1 { + x: u32, + y: u32, + z: u32, +} + +#[repr(C)] +pub struct FooOversize2 { + f0: u16, + f1: u16, + f2: u16, + f3: u16, + f4: u16, +} + +#[repr(C)] +pub struct FooFloat1 { + x: f32, + y: f32, +} + +#[repr(C)] +pub struct FooFloat2 { + x: f64, +} + +#[repr(C)] +pub struct FooFloat3 { + x: f32, +} + +pub mod tests { + use { + Foo, Foo1, Foo2, Foo3, Foo4, Foo5, FooFloat1, FooFloat2, FooFloat3, FooOversize1, + FooOversize2, + }; + + // ENABLED: i64 @f1() + // DISABLED: void @f1(ptr {{.*}}sret + #[no_mangle] + pub extern "fastcall" fn f1() -> Foo { + Foo { x: 1, y: 2 } + } + + // CHECK: { i32, i32 } @f2() + #[no_mangle] + pub extern "Rust" fn f2() -> Foo { + Foo { x: 1, y: 2 } + } + + // ENABLED: i64 @f3() + // DISABLED: void @f3(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f3() -> Foo { + Foo { x: 1, y: 2 } + } + + // ENABLED: i64 @f4() + // DISABLED: void @f4(ptr {{.*}}sret + #[no_mangle] + pub extern "cdecl" fn f4() -> Foo { + Foo { x: 1, y: 2 } + } + + // ENABLED: i64 @f5() + // DISABLED: void @f5(ptr {{.*}}sret + #[no_mangle] + pub extern "stdcall" fn f5() -> Foo { + Foo { x: 1, y: 2 } + } + + // ENABLED: i64 @f6() + // DISABLED: void @f6(ptr {{.*}}sret + #[no_mangle] + pub extern "thiscall" fn f6() -> Foo { + Foo { x: 1, y: 2 } + } + + // ENABLED: i32 @f7() + // DISABLED: void @f7(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f7() -> Foo1 { + Foo1 { x: 1 } + } + + // ENABLED: i32 @f8() + // DISABLED: void @f8(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f8() -> Foo2 { + Foo2 { x: true, y: false, z: 5 } + } + + // ENABLED: i32 @f9() + // DISABLED: void @f9(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f9() -> Foo3 { + Foo3 { x: 5, y: false, z: true } + } + + // ENABLED: i64 @f10() + // DISABLED: void @f10(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f10() -> Foo4 { + Foo4 { x: 'x', y: true, z: 170 } + } + + // ENABLED: i64 @f11() + // DISABLED: void @f11(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f11() -> Foo5 { + Foo5 { x: 1, y: 2, z: 3, a: true } + } + + // CHECK: void @f12(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f12() -> FooOversize1 { + FooOversize1 { x: 1, y: 2, z: 3 } + } + + // CHECK: void @f13(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f13() -> FooOversize2 { + FooOversize2 { f0: 1, f1: 2, f2: 3, f3: 4, f4: 5 } + } + + // ENABLED: i64 @f14() + // DISABLED: void @f14(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f14() -> FooFloat1 { + FooFloat1 { x: 1.0, y: 1.0 } + } + + // ENABLED: double @f15() + // DISABLED: void @f15(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f15() -> FooFloat2 { + FooFloat2 { x: 1.0 } + } + + // ENABLED: float @f16() + // DISABLED: void @f16(ptr {{.*}}sret + #[no_mangle] + pub extern "C" fn f16() -> FooFloat3 { + FooFloat3 { x: 1.0 } + } +} diff --git a/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.aarch64.stderr b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.aarch64.stderr new file mode 100644 index 0000000000000..9bc85cc7e62d8 --- /dev/null +++ b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.aarch64.stderr @@ -0,0 +1,4 @@ +error: `-Zreg-struct-return` is only supported on x86 + +error: aborting due to 1 previous error + diff --git a/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs new file mode 100644 index 0000000000000..b5e34bece3200 --- /dev/null +++ b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs @@ -0,0 +1,21 @@ +//@ revisions: x86 x86_64 aarch64 + +//@ compile-flags: -Zreg-struct-return + +//@[x86] check-pass +//@[x86] needs-llvm-components: x86 +//@[x86] compile-flags: --target i686-unknown-linux-gnu + +//@[x86_64] check-fail +//@[x86_64] needs-llvm-components: x86 +//@[x86_64] compile-flags: --target x86_64-unknown-linux-gnu +//@[x86_64] error-pattern: `-Zreg-struct-return` is only supported on x86 + +//@[aarch64] check-fail +//@[aarch64] needs-llvm-components: aarch64 +//@[aarch64] compile-flags: --target aarch64-unknown-linux-gnu +//@[aarch64] error-pattern: `-Zreg-struct-return` is only supported on x86 + +#![feature(no_core)] +#![no_core] +#![no_main] diff --git a/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.x86_64.stderr b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.x86_64.stderr new file mode 100644 index 0000000000000..9bc85cc7e62d8 --- /dev/null +++ b/tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.x86_64.stderr @@ -0,0 +1,4 @@ +error: `-Zreg-struct-return` is only supported on x86 + +error: aborting due to 1 previous error + From 98bae8af24f465e40cb6e88ab5ef94009e6868a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Thu, 21 Nov 2024 14:09:46 +0100 Subject: [PATCH 2/5] crashes: more tests --- tests/crashes/132765.rs | 12 ++++++++++++ tests/crashes/132766.rs | 9 +++++++++ tests/crashes/132882.rs | 13 +++++++++++++ tests/crashes/132981.rs | 5 +++++ tests/crashes/133063.rs | 8 ++++++++ tests/crashes/133066.rs | 12 ++++++++++++ tests/crashes/133199.rs | 11 +++++++++++ tests/crashes/133275-1.rs | 13 +++++++++++++ tests/crashes/133275-2.rs | 15 +++++++++++++++ tests/crashes/auxiliary/aux133199.rs | 13 +++++++++++++ 10 files changed, 111 insertions(+) create mode 100644 tests/crashes/132765.rs create mode 100644 tests/crashes/132766.rs create mode 100644 tests/crashes/132882.rs create mode 100644 tests/crashes/132981.rs create mode 100644 tests/crashes/133063.rs create mode 100644 tests/crashes/133066.rs create mode 100644 tests/crashes/133199.rs create mode 100644 tests/crashes/133275-1.rs create mode 100644 tests/crashes/133275-2.rs create mode 100644 tests/crashes/auxiliary/aux133199.rs diff --git a/tests/crashes/132765.rs b/tests/crashes/132765.rs new file mode 100644 index 0000000000000..01e8fdaacff77 --- /dev/null +++ b/tests/crashes/132765.rs @@ -0,0 +1,12 @@ +//@ known-bug: #132765 + +trait LendingIterator { + type Item<'q>; + fn for_each(&self, _f: Box)>) {} +} + +fn f(_: ()) {} + +fn main() { + LendingIterator::for_each(&(), f); +} diff --git a/tests/crashes/132766.rs b/tests/crashes/132766.rs new file mode 100644 index 0000000000000..5f5117d77a521 --- /dev/null +++ b/tests/crashes/132766.rs @@ -0,0 +1,9 @@ +//@ known-bug: #132766 + +trait Trait {} +impl<'a> Trait for () { + fn pass2<'a>() -> impl Trait2 {} +} + +trait Trait2 {} +impl Trait2 for () {} diff --git a/tests/crashes/132882.rs b/tests/crashes/132882.rs new file mode 100644 index 0000000000000..6b5e4dba803b0 --- /dev/null +++ b/tests/crashes/132882.rs @@ -0,0 +1,13 @@ +//@ known-bug: #132882 + +use std::ops::Add; + +pub trait Numoid +where + for &'a Self: Add, +{ +} + +pub fn compute(a: N) -> N { + &a + a +} diff --git a/tests/crashes/132981.rs b/tests/crashes/132981.rs new file mode 100644 index 0000000000000..36808ac91e2fc --- /dev/null +++ b/tests/crashes/132981.rs @@ -0,0 +1,5 @@ +//@ known-bug: #132981 +//@compile-flags: -Clink-dead-code=true --crate-type lib + +#![feature(rust_cold_cc)] +pub extern "rust-cold" fn foo(_: [usize; 3]) {} diff --git a/tests/crashes/133063.rs b/tests/crashes/133063.rs new file mode 100644 index 0000000000000..132b5486170a8 --- /dev/null +++ b/tests/crashes/133063.rs @@ -0,0 +1,8 @@ +//@ known-bug: #133063 + +fn foo(x: !) { + match x { + (! | !) if false => {} + _ => {} + } +} diff --git a/tests/crashes/133066.rs b/tests/crashes/133066.rs new file mode 100644 index 0000000000000..732ebb7079fd0 --- /dev/null +++ b/tests/crashes/133066.rs @@ -0,0 +1,12 @@ +//@ known-bug: #133066 +trait Owner { + const C: u32; +} + +impl Owner for () {;} + +fn take0(_: impl Owner = { N }>) {} + +fn main() { + take0::(()); +} diff --git a/tests/crashes/133199.rs b/tests/crashes/133199.rs new file mode 100644 index 0000000000000..76535fa83a6d0 --- /dev/null +++ b/tests/crashes/133199.rs @@ -0,0 +1,11 @@ +//@ known-bug: #133199 +//@ aux-build: aux133199.rs + +extern crate aux133199; + +use aux133199::FixedBitSet; + +fn main() { + FixedBitSet::<7>::new(); + //~^ ERROR +} diff --git a/tests/crashes/133275-1.rs b/tests/crashes/133275-1.rs new file mode 100644 index 0000000000000..73c04f5d6e410 --- /dev/null +++ b/tests/crashes/133275-1.rs @@ -0,0 +1,13 @@ +//@ known-bug: #133275 +#![feature(const_trait_impl)] +#![feature(associated_type_defaults)] + +#[const_trait] +trait Foo3 +where + Self::Baz: Clone, +{ + type Baz = T; +} + +pub fn main() {} diff --git a/tests/crashes/133275-2.rs b/tests/crashes/133275-2.rs new file mode 100644 index 0000000000000..a774b3cdb6905 --- /dev/null +++ b/tests/crashes/133275-2.rs @@ -0,0 +1,15 @@ +//@ known-bug: #133275 +#![feature(const_trait_impl)] +#[const_trait] +pub trait Owo::T> {} + +#[const_trait] +trait Foo3 +where + Self::Bar: Clone, + Self::Baz: Clone, +{ + type Bar = Vec; + type Baz = T; + //~^ ERROR the trait bound `T: Clone` is not satisfied +} diff --git a/tests/crashes/auxiliary/aux133199.rs b/tests/crashes/auxiliary/aux133199.rs new file mode 100644 index 0000000000000..40765d92fbfe9 --- /dev/null +++ b/tests/crashes/auxiliary/aux133199.rs @@ -0,0 +1,13 @@ +#![allow(incomplete_features)] +#![feature(generic_const_exprs)] + +pub struct FixedBitSet; + +impl FixedBitSet +where + [u8; N.div_ceil(8)]: Sized, +{ + pub fn new() -> Self { + todo!() + } +} From b781165a4c6eb00e22f69d7247ab82e47af8062a Mon Sep 17 00:00:00 2001 From: Xelph Date: Tue, 3 Dec 2024 01:23:28 -0700 Subject: [PATCH 3/5] Improve documentation Fix missing newlines that rustfmt removed. fix trailing whitespace Fix duplicate word. Reformat panic reasons into a list remove trailing whitespace 2 electric boogaloo Change verbe tense. Integrate suggestions --- library/alloc/src/vec/mod.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 4b706086e0770..457be3ae77fc6 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -3025,26 +3025,29 @@ impl Vec { self.spec_extend(other.iter()) } - /// Copies elements from `src` range to the end of the vector. + /// Given a range `src`, clones a slice of elements in that range and appends it to the end. + /// + /// `src` must be a range that can form a valid subslice of the `Vec`. /// /// # Panics /// - /// Panics if the starting point is greater than the end point or if - /// the end point is greater than the length of the vector. + /// Panics if starting index is greater than the end index + /// or if the index is greater than the length of the vector. /// /// # Examples /// /// ``` - /// let mut vec = vec![0, 1, 2, 3, 4]; - /// - /// vec.extend_from_within(2..); - /// assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4]); + /// let mut characters = vec!['a', 'b', 'c', 'd', 'e']; + /// characters.extend_from_within(2..); + /// assert_eq!(characters, ['a', 'b', 'c', 'd', 'e', 'c', 'd', 'e']); /// - /// vec.extend_from_within(..2); - /// assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4, 0, 1]); + /// let mut numbers = vec![0, 1, 2, 3, 4]; + /// numbers.extend_from_within(..2); + /// assert_eq!(numbers, [0, 1, 2, 3, 4, 0, 1]); /// - /// vec.extend_from_within(4..8); - /// assert_eq!(vec, [0, 1, 2, 3, 4, 2, 3, 4, 0, 1, 4, 2, 3, 4]); + /// let mut strings = vec![String::from("hello"), String::from("world"), String::from("!")]; + /// strings.extend_from_within(1..=2); + /// assert_eq!(strings, ["hello", "world", "!", "world", "!"]); /// ``` #[cfg(not(no_global_oom_handling))] #[stable(feature = "vec_extend_from_within", since = "1.53.0")] From 74756c1f453bc248b12b6cf443b68ef2d5bbb071 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Thu, 5 Dec 2024 10:10:59 -0700 Subject: [PATCH 4/5] rustbook: update to use new mdbook-trpl package from The Book --- src/doc/book | 2 +- src/tools/rustbook/Cargo.lock | 47 +++++++++++++++++----------------- src/tools/rustbook/Cargo.toml | 3 +-- src/tools/rustbook/src/main.rs | 11 +++++--- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/doc/book b/src/doc/book index 614c19cb40256..9900d976bbfec 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 614c19cb4025636eb2ba68ebb3d44e3bd3a5e6e4 +Subproject commit 9900d976bbfecf4e8124da54351a9ad85ee3c7f3 diff --git a/src/tools/rustbook/Cargo.lock b/src/tools/rustbook/Cargo.lock index 400eb7c5e0d7b..024b8f9becb09 100644 --- a/src/tools/rustbook/Cargo.lock +++ b/src/tools/rustbook/Cargo.lock @@ -709,30 +709,20 @@ dependencies = [ ] [[package]] -name = "mdbook-trpl-listing" +name = "mdbook-trpl" version = "0.1.0" dependencies = [ + "anyhow", "clap", "html_parser", "mdbook", - "pulldown-cmark 0.10.3", - "pulldown-cmark-to-cmark 13.0.0", + "pulldown-cmark 0.12.2", + "pulldown-cmark-to-cmark 19.0.0", "serde_json", "thiserror", "toml 0.8.19", ] -[[package]] -name = "mdbook-trpl-note" -version = "1.0.0" -dependencies = [ - "clap", - "mdbook", - "pulldown-cmark 0.10.3", - "pulldown-cmark-to-cmark 13.0.0", - "serde_json", -] - [[package]] name = "memchr" version = "2.7.4" @@ -1010,7 +1000,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76979bea66e7875e7509c4ec5300112b316af87fa7a252ca91c448b32dfe3993" dependencies = [ "bitflags 2.6.0", - "getopts", "memchr", "pulldown-cmark-escape 0.10.1", "unicase", @@ -1028,6 +1017,19 @@ dependencies = [ "unicase", ] +[[package]] +name = "pulldown-cmark" +version = "0.12.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f86ba2052aebccc42cbbb3ed234b8b13ce76f75c3551a303cb2bcffcff12bb14" +dependencies = [ + "bitflags 2.6.0", + "getopts", + "memchr", + "pulldown-cmark-escape 0.11.0", + "unicase", +] + [[package]] name = "pulldown-cmark-escape" version = "0.10.1" @@ -1042,20 +1044,20 @@ checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae" [[package]] name = "pulldown-cmark-to-cmark" -version = "13.0.0" +version = "15.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f609795c8d835f79dcfcf768415b9fb57ef1b74891e99f86e73f43a7a257163b" +checksum = "b9c77db841443d89a57ae94f22d29c022f6d9f41b00bddbf1f4024dbaf4bdce1" dependencies = [ - "pulldown-cmark 0.10.3", + "pulldown-cmark 0.11.3", ] [[package]] name = "pulldown-cmark-to-cmark" -version = "15.0.1" +version = "19.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9c77db841443d89a57ae94f22d29c022f6d9f41b00bddbf1f4024dbaf4bdce1" +checksum = "7d742adcc7b655dba3e9ebab47954ca229fc0fa1df01fdc94349b6f3a2e6d257" dependencies = [ - "pulldown-cmark 0.11.3", + "pulldown-cmark 0.12.2", ] [[package]] @@ -1144,8 +1146,7 @@ dependencies = [ "mdbook", "mdbook-i18n-helpers", "mdbook-spec", - "mdbook-trpl-listing", - "mdbook-trpl-note", + "mdbook-trpl", ] [[package]] diff --git a/src/tools/rustbook/Cargo.toml b/src/tools/rustbook/Cargo.toml index 854c454733759..c2ce8fef4d0b8 100644 --- a/src/tools/rustbook/Cargo.toml +++ b/src/tools/rustbook/Cargo.toml @@ -9,8 +9,7 @@ edition = "2021" [dependencies] clap = "4.0.32" env_logger = "0.11" -mdbook-trpl-listing = { path = "../../doc/book/packages/mdbook-trpl-listing" } -mdbook-trpl-note = { path = "../../doc/book/packages/mdbook-trpl-note" } +mdbook-trpl = { path = "../../doc/book/packages/mdbook-trpl" } mdbook-i18n-helpers = "0.3.3" mdbook-spec = { path = "../../doc/reference/mdbook-spec" } diff --git a/src/tools/rustbook/src/main.rs b/src/tools/rustbook/src/main.rs index a1ef18610b000..33f2a51215dfe 100644 --- a/src/tools/rustbook/src/main.rs +++ b/src/tools/rustbook/src/main.rs @@ -6,8 +6,7 @@ use mdbook::MDBook; use mdbook::errors::Result as Result3; use mdbook_i18n_helpers::preprocessors::Gettext; use mdbook_spec::Spec; -use mdbook_trpl_listing::TrplListing; -use mdbook_trpl_note::TrplNote; +use mdbook_trpl::{Figure, Listing, Note}; fn main() { let crate_version = concat!("v", crate_version!()); @@ -109,11 +108,15 @@ pub fn build(args: &ArgMatches) -> Result3<()> { // preprocessor, or this should modify the config and use // MDBook::load_with_config. if book.config.get_preprocessor("trpl-note").is_some() { - book.with_preprocessor(TrplNote); + book.with_preprocessor(Note); } if book.config.get_preprocessor("trpl-listing").is_some() { - book.with_preprocessor(TrplListing); + book.with_preprocessor(Listing); + } + + if book.config.get_preprocessor("trpl-figure").is_some() { + book.with_preprocessor(Figure); } if book.config.get_preprocessor("spec").is_some() { From 712ceaba35967f4ebc272634f417b5f5400601f8 Mon Sep 17 00:00:00 2001 From: Strophox Date: Mon, 30 Sep 2024 20:29:05 +0200 Subject: [PATCH 5/5] extend Miri to correctly pass mutable pointers through FFI Co-authored-by: Ralf Jung --- .../src/const_eval/dummy_machine.rs | 6 +- .../src/const_eval/machine.rs | 10 +- .../rustc_const_eval/src/interpret/cast.rs | 2 +- .../rustc_const_eval/src/interpret/machine.rs | 8 +- .../rustc_const_eval/src/interpret/memory.rs | 46 ++++ .../src/mir/interpret/allocation.rs | 22 ++ .../interpret/allocation/provenance_map.rs | 19 ++ .../rustc_middle/src/mir/interpret/pointer.rs | 9 + src/tools/miri/src/alloc_addresses/mod.rs | 10 +- src/tools/miri/src/borrow_tracker/mod.rs | 4 +- .../src/borrow_tracker/stacked_borrows/mod.rs | 4 +- .../src/borrow_tracker/tree_borrows/mod.rs | 4 +- src/tools/miri/src/machine.rs | 7 +- src/tools/miri/src/shims/native_lib.rs | 50 +++-- .../tests/native-lib/pass/ptr_read_access.rs | 27 +-- .../tests/native-lib/pass/ptr_write_access.rs | 208 ++++++++++++++++++ .../miri/tests/native-lib/ptr_read_access.c | 8 +- .../miri/tests/native-lib/ptr_write_access.c | 90 ++++++++ src/tools/miri/tests/ui.rs | 1 + 19 files changed, 476 insertions(+), 59 deletions(-) create mode 100644 src/tools/miri/tests/native-lib/pass/ptr_write_access.rs create mode 100644 src/tools/miri/tests/native-lib/ptr_write_access.c diff --git a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index e49d702127de5..817acfcca74bc 100644 --- a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs @@ -168,9 +168,9 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine { }) } - fn expose_ptr( - _ecx: &mut InterpCx<'tcx, Self>, - _ptr: interpret::Pointer, + fn expose_provenance( + _ecx: &InterpCx<'tcx, Self>, + _provenance: Self::Provenance, ) -> interpret::InterpResult<'tcx> { unimplemented!() } diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 19c3195aaa4a3..e5534da8fea9e 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -21,9 +21,8 @@ use crate::errors::{LongRunning, LongRunningWarn}; use crate::fluent_generated as fluent; use crate::interpret::{ self, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame, GlobalAlloc, ImmTy, - InterpCx, InterpResult, MPlaceTy, OpTy, Pointer, RangeSet, Scalar, compile_time_machine, - interp_ok, throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup, - throw_unsup_format, + InterpCx, InterpResult, MPlaceTy, OpTy, RangeSet, Scalar, compile_time_machine, interp_ok, + throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup, throw_unsup_format, }; /// When hitting this many interpreted terminators we emit a deny by default lint @@ -586,7 +585,10 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { } #[inline(always)] - fn expose_ptr(_ecx: &mut InterpCx<'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> { + fn expose_provenance( + _ecx: &InterpCx<'tcx, Self>, + _provenance: Self::Provenance, + ) -> InterpResult<'tcx> { // This is only reachable with -Zunleash-the-miri-inside-of-you. throw_unsup_format!("exposing pointers is not possible at compile-time") } diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 2d1bb5c955172..1ad3283383bb6 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -238,7 +238,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let scalar = src.to_scalar(); let ptr = scalar.to_pointer(self)?; match ptr.into_pointer_or_addr() { - Ok(ptr) => M::expose_ptr(self, ptr)?, + Ok(ptr) => M::expose_provenance(self, ptr.provenance)?, Err(_) => {} // Do nothing, exposing an invalid pointer (`None` provenance) is a NOP. }; interp_ok(ImmTy::from_scalar( diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index dbe09d55b2d3c..a180d5da9418b 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -327,11 +327,11 @@ pub trait Machine<'tcx>: Sized { addr: u64, ) -> InterpResult<'tcx, Pointer>>; - /// Marks a pointer as exposed, allowing it's provenance + /// Marks a pointer as exposed, allowing its provenance /// to be recovered. "Pointer-to-int cast" - fn expose_ptr( - ecx: &mut InterpCx<'tcx, Self>, - ptr: Pointer, + fn expose_provenance( + ecx: &InterpCx<'tcx, Self>, + provenance: Self::Provenance, ) -> InterpResult<'tcx>; /// Convert a pointer with provenance into an allocation-offset pair and extra provenance info. diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 07566e9fda274..a53c376265672 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -944,6 +944,52 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { interp_ok(()) } + /// Handle the effect an FFI call might have on the state of allocations. + /// This overapproximates the modifications which external code might make to memory: + /// We set all reachable allocations as initialized, mark all provenances as exposed + /// and overwrite them with `Provenance::WILDCARD`. + pub fn prepare_for_native_call( + &mut self, + id: AllocId, + initial_prov: M::Provenance, + ) -> InterpResult<'tcx> { + // Expose provenance of the root allocation. + M::expose_provenance(self, initial_prov)?; + + let mut done = FxHashSet::default(); + let mut todo = vec![id]; + while let Some(id) = todo.pop() { + if !done.insert(id) { + // We already saw this allocation before, don't process it again. + continue; + } + let info = self.get_alloc_info(id); + + // If there is no data behind this pointer, skip this. + if !matches!(info.kind, AllocKind::LiveData) { + continue; + } + + // Expose all provenances in this allocation, and add them to `todo`. + let alloc = self.get_alloc_raw(id)?; + for prov in alloc.provenance().provenances() { + M::expose_provenance(self, prov)?; + if let Some(id) = prov.get_alloc_id() { + todo.push(id); + } + } + + // Prepare for possible write from native code if mutable. + if info.mutbl.is_mut() { + self.get_alloc_raw_mut(id)? + .0 + .prepare_for_native_write() + .map_err(|e| e.to_interp_error(id))?; + } + } + interp_ok(()) + } + /// Create a lazy debug printer that prints the given allocation and all allocations it points /// to, recursively. #[must_use] diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 509f2667b35b2..d6f8fed755f08 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -643,6 +643,28 @@ impl Allocation Ok(()) } + /// Initialize all previously uninitialized bytes in the entire allocation, and set + /// provenance of everything to `Wildcard`. Before calling this, make sure all + /// provenance in this allocation is exposed! + pub fn prepare_for_native_write(&mut self) -> AllocResult { + let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) }; + // Overwrite uninitialized bytes with 0, to ensure we don't leak whatever their value happens to be. + for chunk in self.init_mask.range_as_init_chunks(full_range) { + if !chunk.is_init() { + let uninit_bytes = &mut self.bytes + [chunk.range().start.bytes_usize()..chunk.range().end.bytes_usize()]; + uninit_bytes.fill(0); + } + } + // Mark everything as initialized now. + self.mark_init(full_range, true); + + // Set provenance of all bytes to wildcard. + self.provenance.write_wildcards(self.len()); + + Ok(()) + } + /// Remove all provenance in the given memory range. pub fn clear_provenance(&mut self, cx: &impl HasDataLayout, range: AllocRange) -> AllocResult { self.provenance.clear(range, cx)?; diff --git a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index 5c47fc6a399e4..3a83b184d83d5 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs @@ -195,6 +195,25 @@ impl ProvenanceMap { Ok(()) } + + /// Overwrites all provenance in the allocation with wildcard provenance. + /// + /// Provided for usage in Miri and panics otherwise. + pub fn write_wildcards(&mut self, alloc_size: usize) { + assert!( + Prov::OFFSET_IS_ADDR, + "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false" + ); + let wildcard = Prov::WILDCARD.unwrap(); + + // Remove all pointer provenances, then write wildcards into the whole byte range. + self.ptrs.clear(); + let last = Size::from_bytes(alloc_size); + let bytes = self.bytes.get_or_insert_with(Box::default); + for offset in Size::ZERO..last { + bytes.insert(offset, wildcard); + } + } } /// A partial, owned list of provenance to transfer into another allocation. diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index 1d5afe22573e0..25c7c26ddd974 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -66,6 +66,9 @@ pub trait Provenance: Copy + fmt::Debug + 'static { /// pointer, and implement ptr-to-int transmutation by stripping provenance. const OFFSET_IS_ADDR: bool; + /// If wildcard provenance is implemented, contains the unique, general wildcard provenance variant. + const WILDCARD: Option; + /// Determines how a pointer should be printed. fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result; @@ -168,6 +171,9 @@ impl Provenance for CtfeProvenance { // so ptr-to-int casts are not possible (since we do not know the global physical offset). const OFFSET_IS_ADDR: bool = false; + // `CtfeProvenance` does not implement wildcard provenance. + const WILDCARD: Option = None; + fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Print AllocId. fmt::Debug::fmt(&ptr.provenance.alloc_id(), f)?; // propagates `alternate` flag @@ -197,6 +203,9 @@ impl Provenance for AllocId { // so ptr-to-int casts are not possible (since we do not know the global physical offset). const OFFSET_IS_ADDR: bool = false; + // `AllocId` does not implement wildcard provenance. + const WILDCARD: Option = None; + fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Forward `alternate` flag to `alloc_id` printing. if f.alternate() { diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index fe7d8db245b94..f7295fd7d8a5e 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -286,9 +286,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { - fn expose_ptr(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let global_state = this.machine.alloc_addresses.get_mut(); + fn expose_ptr(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + let mut global_state = this.machine.alloc_addresses.borrow_mut(); // In strict mode, we don't need this, so we can save some cycles by not tracking it. if global_state.provenance_mode == ProvenanceMode::Strict { return interp_ok(()); @@ -299,8 +299,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(()); } trace!("Exposing allocation id {alloc_id:?}"); - let global_state = this.machine.alloc_addresses.get_mut(); global_state.exposed.insert(alloc_id); + // Release the global state before we call `expose_tag`, which may call `get_alloc_info_extra`, + // which may need access to the global state. + drop(global_state); if this.machine.borrow_tracker.is_some() { this.expose_tag(alloc_id, tag)?; } diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 4883613dea504..9808102f4ba66 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -302,8 +302,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - fn expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag), diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 745316913d992..355ed1e0f31b7 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -1011,8 +1011,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. - fn sb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn sb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); // Function pointers and dead objects don't have an alloc_extra so we ignore them. // This is okay because accessing them is UB anyway, no need for any Stacked Borrows checks. diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 255a3578aaec3..17329e7b4b5be 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -532,8 +532,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. - fn tb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn tb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); // Function pointers and dead objects don't have an alloc_extra so we ignore them. // This is okay because accessing them is UB anyway, no need for any Tree Borrows checks. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 9c1951ec87a71..b6f07446be7d7 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -268,6 +268,9 @@ impl interpret::Provenance for Provenance { /// We use absolute addresses in the `offset` of a `StrictPointer`. const OFFSET_IS_ADDR: bool = true; + /// Miri implements wildcard provenance. + const WILDCARD: Option = Some(Provenance::Wildcard); + fn get_alloc_id(self) -> Option { match self { Provenance::Concrete { alloc_id, .. } => Some(alloc_id), @@ -1241,8 +1244,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { /// Called on `ptr as usize` casts. /// (Actually computing the resulting `usize` doesn't need machine help, /// that's just `Scalar::try_to_int`.) - fn expose_ptr(ecx: &mut InterpCx<'tcx, Self>, ptr: StrictPointer) -> InterpResult<'tcx> { - match ptr.provenance { + fn expose_provenance(ecx: &InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> { + match provenance { Provenance::Concrete { alloc_id, tag } => ecx.expose_ptr(alloc_id, tag), Provenance::Wildcard => { // No need to do anything for wildcard pointers as diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index e7a4251242e2a..4082b8eed4597 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -3,8 +3,11 @@ use std::ops::Deref; use libffi::high::call as ffi; use libffi::low::CodePtr; -use rustc_abi::{BackendRepr, HasDataLayout}; -use rustc_middle::ty::{self as ty, IntTy, UintTy}; +use rustc_abi::{BackendRepr, HasDataLayout, Size}; +use rustc_middle::{ + mir::interpret::Pointer, + ty::{self as ty, IntTy, UintTy}, +}; use rustc_span::Symbol; use crate::*; @@ -75,6 +78,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { unsafe { ffi::call::<()>(ptr, libffi_args.as_slice()) }; return interp_ok(ImmTy::uninit(dest.layout)); } + ty::RawPtr(..) => { + let x = unsafe { ffi::call::<*const ()>(ptr, libffi_args.as_slice()) }; + let ptr = Pointer::new(Provenance::Wildcard, Size::from_bytes(x.addr())); + Scalar::from_pointer(ptr, this) + } _ => throw_unsup_format!("unsupported return type for native call: {:?}", link_name), }; interp_ok(ImmTy::from_scalar(scalar, dest.layout)) @@ -152,8 +160,26 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if !matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) { throw_unsup_format!("only scalar argument types are support for native calls") } - libffi_args.push(imm_to_carg(this.read_immediate(arg)?, this)?); + let imm = this.read_immediate(arg)?; + libffi_args.push(imm_to_carg(&imm, this)?); + // If we are passing a pointer, prepare the memory it points to. + if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) { + let ptr = imm.to_scalar().to_pointer(this)?; + let Some(prov) = ptr.provenance else { + // Pointer without provenance may not access any memory. + continue; + }; + // We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance. + let Some(alloc_id) = prov.get_alloc_id() else { + // Wildcard pointer, whatever it points to must be already exposed. + continue; + }; + this.prepare_for_native_call(alloc_id, prov)?; + } } + + // FIXME: In the future, we should also call `prepare_for_native_call` on all previously + // exposed allocations, since C may access any of them. // Convert them to `libffi::high::Arg` type. let libffi_args = libffi_args @@ -220,7 +246,7 @@ impl<'a> CArg { /// Extract the scalar value from the result of reading a scalar from the machine, /// and convert it to a `CArg`. -fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> { +fn imm_to_carg<'tcx>(v: &ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> { interp_ok(match v.layout.ty.kind() { // If the primitive provided can be converted to a type matching the type pattern // then create a `CArg` of this primitive value with the corresponding `CArg` constructor. @@ -238,18 +264,10 @@ fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'t ty::Uint(UintTy::U64) => CArg::UInt64(v.to_scalar().to_u64()?), ty::Uint(UintTy::Usize) => CArg::USize(v.to_scalar().to_target_usize(cx)?.try_into().unwrap()), - ty::RawPtr(_, mutability) => { - // Arbitrary mutable pointer accesses are not currently supported in Miri. - if mutability.is_mut() { - throw_unsup_format!( - "unsupported mutable pointer type for native call: {}", - v.layout.ty - ); - } else { - let s = v.to_scalar().to_pointer(cx)?.addr(); - // This relies on the `expose_provenance` in `addr_from_alloc_id`. - CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize())) - } + ty::RawPtr(..) => { + let s = v.to_scalar().to_pointer(cx)?.addr(); + // This relies on the `expose_provenance` in `addr_from_alloc_id`. + CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize())) } _ => throw_unsup_format!("unsupported argument type for native call: {}", v.layout.ty), }) diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index 46eb5778b3258..3ccfecc6fb379 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -3,17 +3,14 @@ //@only-on-host fn main() { - test_pointer(); - - test_simple(); - - test_nested(); - - test_static(); + test_access_pointer(); + test_access_simple(); + test_access_nested(); + test_access_static(); } -// Test void function that dereferences a pointer and prints its contents from C. -fn test_pointer() { +/// Test function that dereferences an int pointer and prints its contents from C. +fn test_access_pointer() { extern "C" { fn print_pointer(ptr: *const i32); } @@ -23,8 +20,8 @@ fn test_pointer() { unsafe { print_pointer(&x) }; } -// Test function that dereferences a simple struct pointer and accesses a field. -fn test_simple() { +/// Test function that dereferences a simple struct pointer and accesses a field. +fn test_access_simple() { #[repr(C)] struct Simple { field: i32, @@ -39,8 +36,8 @@ fn test_simple() { assert_eq!(unsafe { access_simple(&simple) }, -42); } -// Test function that dereferences nested struct pointers and accesses fields. -fn test_nested() { +/// Test function that dereferences nested struct pointers and accesses fields. +fn test_access_nested() { use std::ptr::NonNull; #[derive(Debug, PartialEq, Eq)] @@ -61,8 +58,8 @@ fn test_nested() { assert_eq!(unsafe { access_nested(&nested_2) }, 97); } -// Test function that dereferences static struct pointers and accesses fields. -fn test_static() { +/// Test function that dereferences a static struct pointer and accesses fields. +fn test_access_static() { #[repr(C)] struct Static { value: i32, diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs new file mode 100644 index 0000000000000..4045ef3cee5ce --- /dev/null +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -0,0 +1,208 @@ +// Only works on Unix targets +//@ignore-target: windows wasm +//@only-on-host +//@compile-flags: -Zmiri-permissive-provenance + + +#![feature(box_as_ptr)] + +use std::mem::MaybeUninit; +use std::ptr::null; + +fn main() { + test_increment_int(); + test_init_int(); + test_init_array(); + test_init_static_inner(); + test_exposed(); + test_swap_ptr(); + test_swap_ptr_tuple(); + test_overwrite_dangling(); + test_pass_dangling(); + test_swap_ptr_triple_dangling(); + test_return_ptr(); +} + +/// Test function that modifies an int. +fn test_increment_int() { + extern "C" { + fn increment_int(ptr: *mut i32); + } + + let mut x = 11; + + unsafe { increment_int(&mut x) }; + assert_eq!(x, 12); +} + +/// Test function that initializes an int. +fn test_init_int() { + extern "C" { + fn init_int(ptr: *mut i32, val: i32); + } + + let mut x = MaybeUninit::::uninit(); + let val = 21; + + let x = unsafe { + init_int(x.as_mut_ptr(), val); + x.assume_init() + }; + assert_eq!(x, val); +} + +/// Test function that initializes an array. +fn test_init_array() { + extern "C" { + fn init_array(ptr: *mut i32, len: usize, val: i32); + } + + const LEN: usize = 3; + let mut array = MaybeUninit::<[i32; LEN]>::uninit(); + let val = 31; + + let array = unsafe { + init_array(array.as_mut_ptr().cast::(), LEN, val); + array.assume_init() + }; + assert_eq!(array, [val; LEN]); +} + +/// Test function that initializes an int pointed to by an immutable static. +fn test_init_static_inner() { + #[repr(C)] + struct SyncPtr { + ptr: *mut i32 + } + unsafe impl Sync for SyncPtr {} + + extern "C" { + fn init_static_inner(s_ptr: *const SyncPtr, val: i32); + } + + static mut INNER: MaybeUninit = MaybeUninit::uninit(); + #[allow(static_mut_refs)] + static STATIC: SyncPtr = SyncPtr { ptr: unsafe { INNER.as_mut_ptr() } }; + let val = 41; + + let inner = unsafe { + init_static_inner(&STATIC, val); + INNER.assume_init() + }; + assert_eq!(inner, val); +} + +// Test function that marks an allocation as exposed. +fn test_exposed() { + extern "C" { + fn ignore_ptr(ptr: *const i32); + } + + let x = 51; + let ptr = &raw const x; + let p = ptr.addr(); + + unsafe { ignore_ptr(ptr) }; + assert_eq!(unsafe { *(p as *const i32) }, x); +} + +/// Test function that swaps two pointers and exposes the alloc of an int. +fn test_swap_ptr() { + extern "C" { + fn swap_ptr(pptr0: *mut *const i32, pptr1: *mut *const i32); + } + + let x = 61; + let (mut ptr0, mut ptr1) = (&raw const x, null()); + + unsafe { swap_ptr(&mut ptr0, &mut ptr1) }; + assert_eq!(unsafe { *ptr1 }, x); +} + +/// Test function that swaps two pointers in a struct and exposes the alloc of an int. +fn test_swap_ptr_tuple() { + #[repr(C)] + struct Tuple { + ptr0: *const i32, + ptr1: *const i32, + } + + extern "C" { + fn swap_ptr_tuple(t_ptr: *mut Tuple); + } + + let x = 71; + let mut tuple = Tuple { ptr0: &raw const x, ptr1: null() }; + + unsafe { swap_ptr_tuple(&mut tuple) } + assert_eq!(unsafe { *tuple.ptr1 }, x); +} + +/// Test function that interacts with a dangling pointer. +fn test_overwrite_dangling() { + extern "C" { + fn overwrite_ptr(pptr: *mut *const i32); + } + + let b = Box::new(81); + let mut ptr = Box::as_ptr(&b); + drop(b); + + unsafe { overwrite_ptr(&mut ptr) }; + assert_eq!(ptr, null()); +} + +/// Test function that passes a dangling pointer. +fn test_pass_dangling() { + extern "C" { + fn ignore_ptr(ptr: *const i32); + } + + let b = Box::new(91); + let ptr = Box::as_ptr(&b); + drop(b); + + unsafe { ignore_ptr(ptr) }; +} + +/// Test function that interacts with a struct storing a dangling pointer. +fn test_swap_ptr_triple_dangling() { + #[repr(C)] + struct Triple { + ptr0: *const i32, + ptr1: *const i32, + ptr2: *const i32, + } + + extern "C" { + fn swap_ptr_triple_dangling(t_ptr: *const Triple); + } + + let x = 101; + let b = Box::new(111); + let ptr = Box::as_ptr(&b); + drop(b); + let z = 121; + let triple = Triple { + ptr0: &raw const x, + ptr1: ptr, + ptr2: &raw const z + }; + + unsafe { swap_ptr_triple_dangling(&triple) } + assert_eq!(unsafe { *triple.ptr2 }, x); +} + + +/// Test function that directly returns its pointer argument. +fn test_return_ptr() { + extern "C" { + fn return_ptr(ptr: *const i32) -> *const i32; + } + + let x = 131; + let ptr = &raw const x; + + let ptr = unsafe { return_ptr(ptr) }; + assert_eq!(unsafe { *ptr }, x); +} diff --git a/src/tools/miri/tests/native-lib/ptr_read_access.c b/src/tools/miri/tests/native-lib/ptr_read_access.c index 540845d53a744..3b427d6033e3d 100644 --- a/src/tools/miri/tests/native-lib/ptr_read_access.c +++ b/src/tools/miri/tests/native-lib/ptr_read_access.c @@ -3,13 +3,13 @@ // See comments in build_native_lib() #define EXPORT __attribute__((visibility("default"))) -/* Test: test_pointer */ +/* Test: test_access_pointer */ EXPORT void print_pointer(const int *ptr) { printf("printing pointer dereference from C: %d\n", *ptr); } -/* Test: test_simple */ +/* Test: test_access_simple */ typedef struct Simple { int field; @@ -19,7 +19,7 @@ EXPORT int access_simple(const Simple *s_ptr) { return s_ptr->field; } -/* Test: test_nested */ +/* Test: test_access_nested */ typedef struct Nested { int value; @@ -38,7 +38,7 @@ EXPORT int access_nested(const Nested *n_ptr) { return n_ptr->value; } -/* Test: test_static */ +/* Test: test_access_static */ typedef struct Static { int value; diff --git a/src/tools/miri/tests/native-lib/ptr_write_access.c b/src/tools/miri/tests/native-lib/ptr_write_access.c new file mode 100644 index 0000000000000..b54c5d86b210d --- /dev/null +++ b/src/tools/miri/tests/native-lib/ptr_write_access.c @@ -0,0 +1,90 @@ +#include + +// See comments in build_native_lib() +#define EXPORT __attribute__((visibility("default"))) + +/* Test: test_increment_int */ + +EXPORT void increment_int(int *ptr) { + *ptr += 1; +} + +/* Test: test_init_int */ + +EXPORT void init_int(int *ptr, int val) { + *ptr = val; +} + +/* Test: test_init_array */ + +EXPORT void init_array(int *array, size_t len, int val) { + for (size_t i = 0; i < len; i++) { + array[i] = val; + } +} + +/* Test: test_init_static_inner */ + +typedef struct SyncPtr { + int *ptr; +} SyncPtr; + +EXPORT void init_static_inner(const SyncPtr *s_ptr, int val) { + *(s_ptr->ptr) = val; +} + +/* Tests: test_exposed, test_pass_dangling */ + +EXPORT void ignore_ptr(__attribute__((unused)) const int *ptr) { + return; +} + +/* Test: test_expose_int */ +EXPORT void expose_int(const int *int_ptr, const int **pptr) { + *pptr = int_ptr; +} + +/* Test: test_swap_ptr */ + +EXPORT void swap_ptr(const int **pptr0, const int **pptr1) { + const int *tmp = *pptr0; + *pptr0 = *pptr1; + *pptr1 = tmp; +} + +/* Test: test_swap_ptr_tuple */ + +typedef struct Tuple { + int *ptr0; + int *ptr1; +} Tuple; + +EXPORT void swap_ptr_tuple(Tuple *t_ptr) { + int *tmp = t_ptr->ptr0; + t_ptr->ptr0 = t_ptr->ptr1; + t_ptr->ptr1 = tmp; +} + +/* Test: test_overwrite_dangling */ + +EXPORT void overwrite_ptr(const int **pptr) { + *pptr = NULL; +} + +/* Test: test_swap_ptr_triple_dangling */ + +typedef struct Triple { + int *ptr0; + int *ptr1; + int *ptr2; +} Triple; + +EXPORT void swap_ptr_triple_dangling(Triple *t_ptr) { + int *tmp = t_ptr->ptr0; + t_ptr->ptr0 = t_ptr->ptr2; + t_ptr->ptr2 = tmp; +} + +EXPORT const int *return_ptr(const int *ptr) { + return ptr; +} diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 9553a37c9a82f..9b9542b88a962 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -64,6 +64,7 @@ fn build_native_lib() -> PathBuf { // FIXME: Automate gathering of all relevant C source files in the directory. "tests/native-lib/scalar_arguments.c", "tests/native-lib/ptr_read_access.c", + "tests/native-lib/ptr_write_access.c", // Ensure we notice serious problems in the C code. "-Wall", "-Wextra",