From 7913b8543e90602e98092f23be31259841671ff1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 16 Jun 2022 09:37:01 -0400 Subject: [PATCH 01/10] unit tests that inspect LLVM output directly. This relies on a human being to confirm that the entries actually correspond to what is specified in each of the respective ABIs... updated to incorporate feedback: fix x86_64/i686 tests to use correct name for the corresponding llvm component. --- .../some-abis-do-extend-params-to-32-bits.rs | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 src/test/codegen/some-abis-do-extend-params-to-32-bits.rs diff --git a/src/test/codegen/some-abis-do-extend-params-to-32-bits.rs b/src/test/codegen/some-abis-do-extend-params-to-32-bits.rs new file mode 100644 index 0000000000000..7fc34af3da72a --- /dev/null +++ b/src/test/codegen/some-abis-do-extend-params-to-32-bits.rs @@ -0,0 +1,204 @@ +// compile-flags: -Cno-prepopulate-passes + +// revisions:x86_64 i686 aarch64-apple aarch64-windows aarch64-linux arm riscv + +//[x86_64] compile-flags: --target x86_64-unknown-uefi +//[x86_64] needs-llvm-components: x86 +//[i686] compile-flags: --target i686-unknown-linux-musl +//[i686] needs-llvm-components: x86 +//[aarch64-windows] compile-flags: --target aarch64-pc-windows-msvc +//[aarch64-windows] needs-llvm-components: aarch64 +//[aarch64-linux] compile-flags: --target aarch64-unknown-linux-gnu +//[aarch64-linux] needs-llvm-components: aarch64 +//[aarch64-apple] compile-flags: --target aarch64-apple-darwin +//[aarch64-apple] needs-llvm-components: aarch64 +//[arm] compile-flags: --target armv7r-none-eabi +//[arm] needs-llvm-components: arm +//[riscv] compile-flags: --target riscv64gc-unknown-none-elf +//[riscv] needs-llvm-components: riscv + +// See bottom of file for a corresponding C source file that is meant to yield +// equivalent declarations. +#![feature(no_core, lang_items)] +#![crate_type = "lib"] +#![no_std] +#![no_core] + +#[lang="sized"] trait Sized { } +#[lang="freeze"] trait Freeze { } +#[lang="copy"] trait Copy { } + +// The patterns in this file are written in the style of a table to make the +// uniformities and distinctions more apparent. +// +// ZERO/SIGN-EXTENDING TO 32 BITS NON-EXTENDING +// ============================== ======================= +// x86_64: void @c_arg_u8(i8 zeroext %_a) +// i686: void @c_arg_u8(i8 zeroext %_a) +// aarch64-apple: void @c_arg_u8(i8 zeroext %_a) +// aarch64-windows: void @c_arg_u8(i8 %_a) +// aarch64-linux: void @c_arg_u8(i8 %_a) +// arm: void @c_arg_u8(i8 zeroext %_a) +// riscv: void @c_arg_u8(i8 zeroext %_a) +#[no_mangle] pub extern "C" fn c_arg_u8(_a: u8) { } + +// x86_64: void @c_arg_u16(i16 zeroext %_a) +// i686: void @c_arg_u16(i16 zeroext %_a) +// aarch64-apple: void @c_arg_u16(i16 zeroext %_a) +// aarch64-windows: void @c_arg_u16(i16 %_a) +// aarch64-linux: void @c_arg_u16(i16 %_a) +// arm: void @c_arg_u16(i16 zeroext %_a) +// riscv: void @c_arg_u16(i16 zeroext %_a) +#[no_mangle] pub extern "C" fn c_arg_u16(_a: u16) { } + +// x86_64: void @c_arg_u32(i32 %_a) +// i686: void @c_arg_u32(i32 %_a) +// aarch64-apple: void @c_arg_u32(i32 %_a) +// aarch64-windows: void @c_arg_u32(i32 %_a) +// aarch64-linux: void @c_arg_u32(i32 %_a) +// arm: void @c_arg_u32(i32 %_a) +// riscv: void @c_arg_u32(i32 signext %_a) +#[no_mangle] pub extern "C" fn c_arg_u32(_a: u32) { } + +// x86_64: void @c_arg_u64(i64 %_a) +// i686: void @c_arg_u64(i64 %_a) +// aarch64-apple: void @c_arg_u64(i64 %_a) +// aarch64-windows: void @c_arg_u64(i64 %_a) +// aarch64-linux: void @c_arg_u64(i64 %_a) +// arm: void @c_arg_u64(i64 %_a) +// riscv: void @c_arg_u64(i64 %_a) +#[no_mangle] pub extern "C" fn c_arg_u64(_a: u64) { } + +// x86_64: void @c_arg_i8(i8 signext %_a) +// i686: void @c_arg_i8(i8 signext %_a) +// aarch64-apple: void @c_arg_i8(i8 signext %_a) +// aarch64-windows: void @c_arg_i8(i8 %_a) +// aarch64-linux: void @c_arg_i8(i8 %_a) +// arm: void @c_arg_i8(i8 signext %_a) +// riscv: void @c_arg_i8(i8 signext %_a) +#[no_mangle] pub extern "C" fn c_arg_i8(_a: i8) { } + +// x86_64: void @c_arg_i16(i16 signext %_a) +// i686: void @c_arg_i16(i16 signext %_a) +// aarch64-apple: void @c_arg_i16(i16 signext %_a) +// aarch64-windows: void @c_arg_i16(i16 %_a) +// aarch64-linux: void @c_arg_i16(i16 %_a) +// arm: void @c_arg_i16(i16 signext %_a) +// riscv: void @c_arg_i16(i16 signext %_a) +#[no_mangle] pub extern "C" fn c_arg_i16(_a: i16) { } + +// x86_64: void @c_arg_i32(i32 %_a) +// i686: void @c_arg_i32(i32 %_a) +// aarch64-apple: void @c_arg_i32(i32 %_a) +// aarch64-windows: void @c_arg_i32(i32 %_a) +// aarch64-linux: void @c_arg_i32(i32 %_a) +// arm: void @c_arg_i32(i32 %_a) +// riscv: void @c_arg_i32(i32 signext %_a) +#[no_mangle] pub extern "C" fn c_arg_i32(_a: i32) { } + +// x86_64: void @c_arg_i64(i64 %_a) +// i686: void @c_arg_i64(i64 %_a) +// aarch64-apple: void @c_arg_i64(i64 %_a) +// aarch64-windows: void @c_arg_i64(i64 %_a) +// aarch64-linux: void @c_arg_i64(i64 %_a) +// arm: void @c_arg_i64(i64 %_a) +// riscv: void @c_arg_i64(i64 %_a) +#[no_mangle] pub extern "C" fn c_arg_i64(_a: i64) { } + +// x86_64: zeroext i8 @c_ret_u8() +// i686: zeroext i8 @c_ret_u8() +// aarch64-apple: zeroext i8 @c_ret_u8() +// aarch64-windows: i8 @c_ret_u8() +// aarch64-linux: i8 @c_ret_u8() +// arm: zeroext i8 @c_ret_u8() +// riscv: zeroext i8 @c_ret_u8() +#[no_mangle] pub extern "C" fn c_ret_u8() -> u8 { 0 } + +// x86_64: zeroext i16 @c_ret_u16() +// i686: zeroext i16 @c_ret_u16() +// aarch64-apple: zeroext i16 @c_ret_u16() +// aarch64-windows: i16 @c_ret_u16() +// aarch64-linux: i16 @c_ret_u16() +// arm: zeroext i16 @c_ret_u16() +// riscv: zeroext i16 @c_ret_u16() +#[no_mangle] pub extern "C" fn c_ret_u16() -> u16 { 0 } + +// x86_64: i32 @c_ret_u32() +// i686: i32 @c_ret_u32() +// aarch64-apple: i32 @c_ret_u32() +// aarch64-windows: i32 @c_ret_u32() +// aarch64-linux: i32 @c_ret_u32() +// arm: i32 @c_ret_u32() +// riscv: signext i32 @c_ret_u32() +#[no_mangle] pub extern "C" fn c_ret_u32() -> u32 { 0 } + +// x86_64: i64 @c_ret_u64() +// i686: i64 @c_ret_u64() +// aarch64-apple: i64 @c_ret_u64() +// aarch64-windows: i64 @c_ret_u64() +// aarch64-linux: i64 @c_ret_u64() +// arm: i64 @c_ret_u64() +// riscv: i64 @c_ret_u64() +#[no_mangle] pub extern "C" fn c_ret_u64() -> u64 { 0 } + +// x86_64: signext i8 @c_ret_i8() +// i686: signext i8 @c_ret_i8() +// aarch64-apple: signext i8 @c_ret_i8() +// aarch64-windows: i8 @c_ret_i8() +// aarch64-linux: i8 @c_ret_i8() +// arm: signext i8 @c_ret_i8() +// riscv: signext i8 @c_ret_i8() +#[no_mangle] pub extern "C" fn c_ret_i8() -> i8 { 0 } + +// x86_64: signext i16 @c_ret_i16() +// i686: signext i16 @c_ret_i16() +// aarch64-apple: signext i16 @c_ret_i16() +// aarch64-windows: i16 @c_ret_i16() +// aarch64-linux: i16 @c_ret_i16() +// arm: signext i16 @c_ret_i16() +// riscv: signext i16 @c_ret_i16() +#[no_mangle] pub extern "C" fn c_ret_i16() -> i16 { 0 } + +// x86_64: i32 @c_ret_i32() +// i686: i32 @c_ret_i32() +// aarch64-apple: i32 @c_ret_i32() +// aarch64-windows: i32 @c_ret_i32() +// aarch64-linux: i32 @c_ret_i32() +// arm: i32 @c_ret_i32() +// riscv: signext i32 @c_ret_i32() +#[no_mangle] pub extern "C" fn c_ret_i32() -> i32 { 0 } + +// x86_64: i64 @c_ret_i64() +// i686: i64 @c_ret_i64() +// aarch64-apple: i64 @c_ret_i64() +// aarch64-windows: i64 @c_ret_i64() +// aarch64-linux: i64 @c_ret_i64() +// arm: i64 @c_ret_i64() +// riscv: i64 @c_ret_i64() +#[no_mangle] pub extern "C" fn c_ret_i64() -> i64 { 0 } + +const C_SOURCE_FILE: &'static str = r##" +#include +#include +#include + +void c_arg_u8(uint8_t _a) { } +void c_arg_u16(uint16_t _a) { } +void c_arg_u32(uint32_t _a) { } +void c_arg_u64(uint64_t _a) { } + +void c_arg_i8(int8_t _a) { } +void c_arg_i16(int16_t _a) { } +void c_arg_i32(int32_t _a) { } +void c_arg_i64(int64_t _a) { } + +uint8_t c_ret_u8() { return 0; } +uint16_t c_ret_u16() { return 0; } +uint32_t c_ret_u32() { return 0; } +uint64_t c_ret_u64() { return 0; } + +int8_t c_ret_i8() { return 0; } +int16_t c_ret_i16() { return 0; } +int32_t c_ret_i32() { return 0; } +int64_t c_ret_i64() { return 0; } +"##; From b2777aba757dd92042932d79309132bd953e130d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 6 Jun 2022 12:39:58 -0400 Subject: [PATCH 02/10] End-to-end regression test for 97463. incorporated review feedback, with comment explaining why this is calling CC instead of COMPILE_OBJ or NATIVE_STATICLIB. As drive-by, removed some other unnecessary commands from the recipe. --- .../issue-97463-abi-param-passing/Makefile | 12 ++++++ .../issue-97463-abi-param-passing/bad.c | 24 ++++++++++++ .../param_passing.rs | 38 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile create mode 100644 src/test/run-make-fulldeps/issue-97463-abi-param-passing/bad.c create mode 100644 src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs diff --git a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile new file mode 100644 index 0000000000000..b3db6bcb3f636 --- /dev/null +++ b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile @@ -0,0 +1,12 @@ +-include ../tools.mk + +# The issue exercised by this test, rust-lang/rust#97463, explicitly needs `-O` +# flags (like `-O3`) to reproduce. Thus, we call $(CC) instead of nicer +# alternatives provided by tools.mk like using `COMPILE_OBJ` or using a +# `NATIVE_STATICLIB` dependency. + +all: + $(CC) -c -O3 -o $(TMPDIR)/bad.o bad.c + $(AR) rcs $(TMPDIR)/libbad.a $(TMPDIR)/bad.o + $(RUSTC) param_passing.rs -L$(TMPDIR) -lbad -C opt-level=3 + $(call RUN,param_passing) diff --git a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/bad.c b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/bad.c new file mode 100644 index 0000000000000..013314ab20dc4 --- /dev/null +++ b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/bad.c @@ -0,0 +1,24 @@ +#include +#include +#include + + +struct bloc { + uint16_t a; + uint16_t b; + uint16_t c; +}; + +uint16_t c_read_value(uint32_t a, uint32_t b, uint32_t c) { + struct bloc *data = malloc(sizeof(struct bloc)); + + data->a = a & 0xFFFF; + data->b = b & 0xFFFF; + data->c = c & 0xFFFF; + + printf("C struct: a = %u, b = %u, c = %u\n", + (unsigned) data->a, (unsigned) data->b, (unsigned) data->c); + printf("C function returns %u\n", (unsigned) data->b); + + return data->b; /* leak data */ +} diff --git a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs new file mode 100644 index 0000000000000..b4744a3b96e77 --- /dev/null +++ b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs @@ -0,0 +1,38 @@ +// NOTE: Exposing the bug encoded in this test is sensitive to +// LLVM optimization choices. See additional note below for an +// example. + +#[link(name = "bad")] +extern "C" { + pub fn c_read_value(a: u32, b: u32, c: u32) -> u16; +} + +fn main() { + const C1: usize = 0x327b23c6; + const C2: usize = C1 & 0xFFFF; + + let r1: usize = 0x0; + let r2: usize = C1; + let r3: usize = 0x0; + let value: u16 = unsafe { c_read_value(r1 as u32, r2 as u32, r3 as u32) }; + + // NOTE: as an example of the sensitivity of this test to optimization choices, + // uncommenting this block of code makes the bug go away on pnkfeix's machine. + // (But observing via `dbg!` doesn't hide the bug. At least sometimes.) + /* + println!("{}", value); + println!("{}", value as usize); + println!("{}", usize::from(value)); + println!("{}", (value as usize) & 0xFFFF); + */ + + let d1 = value; + let d2 = value as usize; + let d3 = usize::from(value); + let d4 = (value as usize) & 0xFFFF; + + let d = (&d1, &d2, &d3, &d4); + let d_ = (d1, d2, d3, d4); + + assert_eq!(((&(C2 as u16), &C2, &C2, &C2), (C2 as u16, C2, C2, C2)), (d, d_)); +} From 8ae5a55ba55a434995d2e2e87f8ef15237ff8124 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 6 Jun 2022 12:40:10 -0400 Subject: [PATCH 03/10] fix issue 97463 using change suggested by nbdd0121. parameterized on target details to decide value-extension policy on calls, in order to address how Apple's aarch64 ABI differs from that on Linux and Windows. Updated to incorporate review feedback: adjust comment on new enum specifying param extension policy. Updated to incorporate review feedback: shorten enum names and those of its variants to make it less unwieldy. placate tidy. --- compiler/rustc_target/src/abi/call/aarch64.rs | 41 +++++++++++++++---- compiler/rustc_target/src/abi/call/mod.rs | 9 +++- compiler/rustc_target/src/spec/mod.rs | 2 + 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index 4613a459c51d6..b307cc3e0beb1 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -1,6 +1,27 @@ use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform}; use crate::abi::{HasDataLayout, TyAbiInterface}; +/// Given integer-types M and register width N (e.g. M=u16 and N=32 bits), the +/// `ParamExtension` policy specifies how a uM value should be treated when +/// passed via register or stack-slot of width N. See also rust-lang/rust#97463. +#[derive(Copy, Clone, PartialEq)] +pub enum ParamExtension { + /// Indicates that when passing an i8/i16, either as a function argument or + /// as a return value, it must be sign-extended to 32 bits, and likewise a + /// u8/u16 must be zero-extended to 32-bits. (This variant is here to + /// accommodate Apple's deviation from the usual AArch64 ABI as defined by + /// ARM.) + /// + /// See also: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Pass-Arguments-to-Functions-Correctly + ExtendTo32Bits, + + /// Indicates that no sign- nor zero-extension is performed: if a value of + /// type with bitwidth M is passed as function argument or return value, + /// then M bits are copied into the least significant M bits, and the + /// remaining bits of the register (or word of memory) are untouched. + NoExtension, +} + fn is_homogeneous_aggregate<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>) -> Option where Ty: TyAbiInterface<'a, C> + Copy, @@ -24,13 +45,16 @@ where }) } -fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'a, Ty>) +fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'a, Ty>, param_policy: ParamExtension) where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { if !ret.layout.is_aggregate() { - ret.extend_integer_width_to(32); + match param_policy { + ParamExtension::ExtendTo32Bits => ret.extend_integer_width_to(32), + ParamExtension::NoExtension => {} + } return; } if let Some(uniform) = is_homogeneous_aggregate(cx, ret) { @@ -46,13 +70,16 @@ where ret.make_indirect(); } -fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>) +fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, param_policy: ParamExtension) where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { if !arg.layout.is_aggregate() { - arg.extend_integer_width_to(32); + match param_policy { + ParamExtension::ExtendTo32Bits => arg.extend_integer_width_to(32), + ParamExtension::NoExtension => {} + } return; } if let Some(uniform) = is_homogeneous_aggregate(cx, arg) { @@ -68,19 +95,19 @@ where arg.make_indirect(); } -pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) +pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, param_policy: ParamExtension) where Ty: TyAbiInterface<'a, C> + Copy, C: HasDataLayout, { if !fn_abi.ret.is_ignore() { - classify_ret(cx, &mut fn_abi.ret); + classify_ret(cx, &mut fn_abi.ret, param_policy); } for arg in &mut fn_abi.args { if arg.is_ignore() { continue; } - classify_arg(cx, arg); + classify_arg(cx, arg, param_policy); } } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index ca1d1302ec68a..c87c726cb877d 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -687,7 +687,14 @@ impl<'a, Ty> FnAbi<'a, Ty> { } } }, - "aarch64" => aarch64::compute_abi_info(cx, self), + "aarch64" => { + let param_policy = if cx.target_spec().is_like_osx { + aarch64::ParamExtension::ExtendTo32Bits + } else { + aarch64::ParamExtension::NoExtension + }; + aarch64::compute_abi_info(cx, self, param_policy) + } "amdgpu" => amdgpu::compute_abi_info(cx, self), "arm" => arm::compute_abi_info(cx, self), "avr" => avr::compute_abi_info(self), diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index a08603da04095..e78fefdcfad2a 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1250,6 +1250,8 @@ pub struct TargetOptions { pub abi_return_struct_as_int: bool, /// Whether the target toolchain is like macOS's. Only useful for compiling against iOS/macOS, /// in particular running dsymutil and some other stuff like `-dead_strip`. Defaults to false. + /// Also indiates whether to use Apple-specific ABI changes, such as extending function + /// parameters to 32-bits. pub is_like_osx: bool, /// Whether the target toolchain is like Solaris's. /// Only useful for compiling against Illumos/Solaris, From dfdb017a9bb6284f58ff8447cd2a49c778552f62 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 30 Jun 2022 00:31:41 -0400 Subject: [PATCH 04/10] experiment: trying to encode the end-to-end test as a ui test via rust_test_helpers. This instance is almost certainly insufficient because we need to force optimization flags for both the C and Rust sides of the code. but lets find out for sure. --- src/test/auxiliary/rust_test_helpers.c | 12 ++++++ ...sue-97463-broken-abi-leaked-uninit-data.rs | 38 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs diff --git a/src/test/auxiliary/rust_test_helpers.c b/src/test/auxiliary/rust_test_helpers.c index 92b7dd4b7c516..977ea487a9804 100644 --- a/src/test/auxiliary/rust_test_helpers.c +++ b/src/test/auxiliary/rust_test_helpers.c @@ -1,6 +1,7 @@ // Helper functions used only in tests #include +#include #include #include @@ -415,3 +416,14 @@ rust_dbg_unpack_option_u64u64(struct U8TaggedEnumOptionU64U64 o, uint64_t *a, ui return 0; } } + +uint16_t issue_97463_leak_uninit_data(uint32_t a, uint32_t b, uint32_t c) { + struct bloc { uint16_t a; uint16_t b; uint16_t c; }; + struct bloc *data = malloc(sizeof(struct bloc)); + + data->a = a & 0xFFFF; + data->b = b & 0xFFFF; + data->c = c & 0xFFFF; + + return data->b; /* leak data */ +} diff --git a/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs new file mode 100644 index 0000000000000..d04375acb30a2 --- /dev/null +++ b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs @@ -0,0 +1,38 @@ +// run-pass +#![allow(dead_code)] +#![allow(improper_ctypes)] + +#[link(name = "rust_test_helpers", kind = "static")] +extern "C" { + pub fn issue_97463_leak_uninit_data(a: u32, b: u32, c: u32) -> u16; +} + +fn main() { + const C1: usize = 0x327b23c6; + const C2: usize = C1 & 0xFFFF; + + let r1: usize = 0x0; + let r2: usize = C1; + let r3: usize = 0x0; + let value: u16 = unsafe { issue_97463_leak_uninit_data(r1 as u32, r2 as u32, r3 as u32) }; + + // NOTE: as an example of the sensitivity of this test to optimization choices, + // uncommenting this block of code makes the bug go away on pnkfeix's machine. + // (But observing via `dbg!` doesn't hide the bug. At least sometimes.) + /* + println!("{}", value); + println!("{}", value as usize); + println!("{}", usize::from(value)); + println!("{}", (value as usize) & 0xFFFF); + */ + + let d1 = value; + let d2 = value as usize; + let d3 = usize::from(value); + let d4 = (value as usize) & 0xFFFF; + + let d = (&d1, &d2, &d3, &d4); + let d_ = (d1, d2, d3, d4); + + assert_eq!(((&(C2 as u16), &C2, &C2, &C2), (C2 as u16, C2, C2, C2)), (d, d_)); +} From 99c0f91a4dd235824c353a11a2e75c462c9bbb74 Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Thu, 4 Aug 2022 10:41:47 -0400 Subject: [PATCH 05/10] fix typo, thanks wesley Co-authored-by: Wesley Wiser --- .../issue-97463-abi-param-passing/param_passing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs index b4744a3b96e77..c11f3cc72bdf2 100644 --- a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs +++ b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/param_passing.rs @@ -17,7 +17,7 @@ fn main() { let value: u16 = unsafe { c_read_value(r1 as u32, r2 as u32, r3 as u32) }; // NOTE: as an example of the sensitivity of this test to optimization choices, - // uncommenting this block of code makes the bug go away on pnkfeix's machine. + // uncommenting this block of code makes the bug go away on pnkfelix's machine. // (But observing via `dbg!` doesn't hide the bug. At least sometimes.) /* println!("{}", value); From 9bf3d5a82b689484b12bc5cc053d10f632ea6095 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 5 Aug 2022 10:15:59 -0400 Subject: [PATCH 06/10] Ignore test on wasm --- .../ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs index d04375acb30a2..da782f8bbbf81 100644 --- a/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs +++ b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs @@ -1,4 +1,5 @@ // run-pass +// ignore-wasm #![allow(dead_code)] #![allow(improper_ctypes)] From 59cc718e76faee1f844040d60615bacc6bad8643 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 19 Aug 2022 16:15:15 -0400 Subject: [PATCH 07/10] Update codegen tests to accommodate the potential presence/absence of the extension operation depending on target architecture. --- src/test/codegen/abi-repr-ext.rs | 46 ++++++++++++++++++++++-- src/test/codegen/pic-relocation-model.rs | 5 ++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/test/codegen/abi-repr-ext.rs b/src/test/codegen/abi-repr-ext.rs index 2b34eaf94172b..23ade3c7216d3 100644 --- a/src/test/codegen/abi-repr-ext.rs +++ b/src/test/codegen/abi-repr-ext.rs @@ -1,6 +1,32 @@ // compile-flags: -O -#![crate_type="lib"] +// revisions:x86_64 i686 aarch64-apple aarch64-windows aarch64-linux arm riscv + +//[x86_64] compile-flags: --target x86_64-unknown-uefi +//[x86_64] needs-llvm-components: x86 +//[i686] compile-flags: --target i686-unknown-linux-musl +//[i686] needs-llvm-components: x86 +//[aarch64-windows] compile-flags: --target aarch64-pc-windows-msvc +//[aarch64-windows] needs-llvm-components: aarch64 +//[aarch64-linux] compile-flags: --target aarch64-unknown-linux-gnu +//[aarch64-linux] needs-llvm-components: aarch64 +//[aarch64-apple] compile-flags: --target aarch64-apple-darwin +//[aarch64-apple] needs-llvm-components: aarch64 +//[arm] compile-flags: --target armv7r-none-eabi +//[arm] needs-llvm-components: arm +//[riscv] compile-flags: --target riscv64gc-unknown-none-elf +//[riscv] needs-llvm-components: riscv + +// See bottom of file for a corresponding C source file that is meant to yield +// equivalent declarations. +#![feature(no_core, lang_items)] +#![crate_type = "lib"] +#![no_std] +#![no_core] + +#[lang="sized"] trait Sized { } +#[lang="freeze"] trait Freeze { } +#[lang="copy"] trait Copy { } #[repr(i8)] pub enum Type { @@ -8,7 +34,23 @@ pub enum Type { Type2 = 1 } -// CHECK: define{{( dso_local)?}} noundef signext i8 @test() +// To accommodate rust#97800, one might consider writing the below as: +// +// `define{{( dso_local)?}} noundef{{( signext)?}} i8 @test()` +// +// but based on rust#80556, it seems important to actually check for the +// presence of the `signext` for those targets where we expect it. + +// CHECK: define{{( dso_local)?}} noundef +// x86_64-SAME: signext +// aarch64-apple-SAME: signext +// aarch64-windows-NOT: signext +// aarch64-linux-NOT: signext +// arm-SAME: signext +// riscv-SAME: signext +// CHECK-SAME: i8 @test() + + #[no_mangle] pub extern "C" fn test() -> Type { Type::Type1 diff --git a/src/test/codegen/pic-relocation-model.rs b/src/test/codegen/pic-relocation-model.rs index 6e1d5a6c3f271..9b378ecb4e590 100644 --- a/src/test/codegen/pic-relocation-model.rs +++ b/src/test/codegen/pic-relocation-model.rs @@ -10,7 +10,10 @@ pub fn call_foreign_fn() -> u8 { } } -// CHECK: declare zeroext i8 @foreign_fn() +// (Allow but do not require `zeroext` here, because it is not worth effort to +// spell out which targets have it and which ones do not; see rust#97800.) + +// CHECK: declare{{( zeroext)?}} i8 @foreign_fn() extern "C" {fn foreign_fn() -> u8;} // CHECK: !{i32 7, !"PIC Level", i32 2} From ed9b12d7fd567c92c0c438714a19a53f035329d8 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 22 Aug 2022 11:00:54 -0400 Subject: [PATCH 08/10] rustdoc doesn't like bare urls --- compiler/rustc_target/src/abi/call/aarch64.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index b307cc3e0beb1..e88c0cdf0ec3c 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -12,7 +12,7 @@ pub enum ParamExtension { /// accommodate Apple's deviation from the usual AArch64 ABI as defined by /// ARM.) /// - /// See also: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Pass-Arguments-to-Functions-Correctly + /// See also: ExtendTo32Bits, /// Indicates that no sign- nor zero-extension is performed: if a value of From d73614a2eed088b9aeab4ee95fa6fc965609b8e3 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 16 Sep 2022 13:26:22 -0400 Subject: [PATCH 09/10] Do not run run-make test tied to unix-style `$(CC)` on MSVC host. --- .../run-make-fulldeps/issue-97463-abi-param-passing/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile index b3db6bcb3f636..db1b53e152eae 100644 --- a/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile +++ b/src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile @@ -1,5 +1,7 @@ -include ../tools.mk +# ignore-msvc + # The issue exercised by this test, rust-lang/rust#97463, explicitly needs `-O` # flags (like `-O3`) to reproduce. Thus, we call $(CC) instead of nicer # alternatives provided by tools.mk like using `COMPILE_OBJ` or using a From a2de75a8277d7aa265e9a95162a7834552181ede Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 16 Sep 2022 13:26:58 -0400 Subject: [PATCH 10/10] fix typo in comment noted by bjorn3. --- .../ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs index da782f8bbbf81..fba880d4f9a52 100644 --- a/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs +++ b/src/test/ui/abi/issues/issue-97463-broken-abi-leaked-uninit-data.rs @@ -18,7 +18,7 @@ fn main() { let value: u16 = unsafe { issue_97463_leak_uninit_data(r1 as u32, r2 as u32, r3 as u32) }; // NOTE: as an example of the sensitivity of this test to optimization choices, - // uncommenting this block of code makes the bug go away on pnkfeix's machine. + // uncommenting this block of code makes the bug go away on pnkfelix's machine. // (But observing via `dbg!` doesn't hide the bug. At least sometimes.) /* println!("{}", value);