Skip to content

Commit 95a992a

Browse files
borsbors
committed
Auto merge of #97800 - pnkfelix:issue-97463-fix-aarch64-call-abi-does-not-zeroext, r=wesleywiser
Aarch64 call abi does not zeroext (and one cannot assume it does so) Fix #97463
2 parents 4d4e51e + a2de75a commit 95a992a

File tree

11 files changed

+423
-11
lines changed

11 files changed

+423
-11
lines changed

‎compiler/rustc_target/src/abi/call/aarch64.rs

+34-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,27 @@
11
use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform};
22
use crate::abi::{HasDataLayout, TyAbiInterface};
33

4+
/// Given integer-types M and register width N (e.g. M=u16 and N=32 bits), the
5+
/// `ParamExtension` policy specifies how a uM value should be treated when
6+
/// passed via register or stack-slot of width N. See also rust-lang/rust#97463.
7+
#[derive(Copy, Clone, PartialEq)]
8+
pub enum ParamExtension {
9+
/// Indicates that when passing an i8/i16, either as a function argument or
10+
/// as a return value, it must be sign-extended to 32 bits, and likewise a
11+
/// u8/u16 must be zero-extended to 32-bits. (This variant is here to
12+
/// accommodate Apple's deviation from the usual AArch64 ABI as defined by
13+
/// ARM.)
14+
///
15+
/// See also: <https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Pass-Arguments-to-Functions-Correctly>
16+
ExtendTo32Bits,
17+
18+
/// Indicates that no sign- nor zero-extension is performed: if a value of
19+
/// type with bitwidth M is passed as function argument or return value,
20+
/// then M bits are copied into the least significant M bits, and the
21+
/// remaining bits of the register (or word of memory) are untouched.
22+
NoExtension,
23+
}
24+
425
fn is_homogeneous_aggregate<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>) -> Option<Uniform>
526
where
627
Ty: TyAbiInterface<'a, C> + Copy,
@@ -24,13 +45,16 @@ where
2445
})
2546
}
2647

27-
fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'a, Ty>)
48+
fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'a, Ty>, param_policy: ParamExtension)
2849
where
2950
Ty: TyAbiInterface<'a, C> + Copy,
3051
C: HasDataLayout,
3152
{
3253
if !ret.layout.is_aggregate() {
33-
ret.extend_integer_width_to(32);
54+
match param_policy {
55+
ParamExtension::ExtendTo32Bits => ret.extend_integer_width_to(32),
56+
ParamExtension::NoExtension => {}
57+
}
3458
return;
3559
}
3660
if let Some(uniform) = is_homogeneous_aggregate(cx, ret) {
@@ -46,13 +70,16 @@ where
4670
ret.make_indirect();
4771
}
4872

49-
fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>)
73+
fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, param_policy: ParamExtension)
5074
where
5175
Ty: TyAbiInterface<'a, C> + Copy,
5276
C: HasDataLayout,
5377
{
5478
if !arg.layout.is_aggregate() {
55-
arg.extend_integer_width_to(32);
79+
match param_policy {
80+
ParamExtension::ExtendTo32Bits => arg.extend_integer_width_to(32),
81+
ParamExtension::NoExtension => {}
82+
}
5683
return;
5784
}
5885
if let Some(uniform) = is_homogeneous_aggregate(cx, arg) {
@@ -68,19 +95,19 @@ where
6895
arg.make_indirect();
6996
}
7097

71-
pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
98+
pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>, param_policy: ParamExtension)
7299
where
73100
Ty: TyAbiInterface<'a, C> + Copy,
74101
C: HasDataLayout,
75102
{
76103
if !fn_abi.ret.is_ignore() {
77-
classify_ret(cx, &mut fn_abi.ret);
104+
classify_ret(cx, &mut fn_abi.ret, param_policy);
78105
}
79106

80107
for arg in fn_abi.args.iter_mut() {
81108
if arg.is_ignore() {
82109
continue;
83110
}
84-
classify_arg(cx, arg);
111+
classify_arg(cx, arg, param_policy);
85112
}
86113
}

‎compiler/rustc_target/src/abi/call/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,14 @@ impl<'a, Ty> FnAbi<'a, Ty> {
685685
}
686686
}
687687
},
688-
"aarch64" => aarch64::compute_abi_info(cx, self),
688+
"aarch64" => {
689+
let param_policy = if cx.target_spec().is_like_osx {
690+
aarch64::ParamExtension::ExtendTo32Bits
691+
} else {
692+
aarch64::ParamExtension::NoExtension
693+
};
694+
aarch64::compute_abi_info(cx, self, param_policy)
695+
}
689696
"amdgpu" => amdgpu::compute_abi_info(cx, self),
690697
"arm" => arm::compute_abi_info(cx, self),
691698
"avr" => avr::compute_abi_info(self),

‎compiler/rustc_target/src/spec/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1352,6 +1352,8 @@ pub struct TargetOptions {
13521352
pub abi_return_struct_as_int: bool,
13531353
/// Whether the target toolchain is like macOS's. Only useful for compiling against iOS/macOS,
13541354
/// in particular running dsymutil and some other stuff like `-dead_strip`. Defaults to false.
1355+
/// Also indiates whether to use Apple-specific ABI changes, such as extending function
1356+
/// parameters to 32-bits.
13551357
pub is_like_osx: bool,
13561358
/// Whether the target toolchain is like Solaris's.
13571359
/// Only useful for compiling against Illumos/Solaris,

‎src/test/auxiliary/rust_test_helpers.c

+12
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Helper functions used only in tests
22

33
#include <stdint.h>
4+
#include <stdlib.h>
45
#include <assert.h>
56
#include <stdarg.h>
67

@@ -415,3 +416,14 @@ rust_dbg_unpack_option_u64u64(struct U8TaggedEnumOptionU64U64 o, uint64_t *a, ui
415416
return 0;
416417
}
417418
}
419+
420+
uint16_t issue_97463_leak_uninit_data(uint32_t a, uint32_t b, uint32_t c) {
421+
struct bloc { uint16_t a; uint16_t b; uint16_t c; };
422+
struct bloc *data = malloc(sizeof(struct bloc));
423+
424+
data->a = a & 0xFFFF;
425+
data->b = b & 0xFFFF;
426+
data->c = c & 0xFFFF;
427+
428+
return data->b; /* leak data */
429+
}

‎src/test/codegen/abi-repr-ext.rs

+44-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,56 @@
11
// compile-flags: -O
22

3-
#![crate_type="lib"]
3+
// revisions:x86_64 i686 aarch64-apple aarch64-windows aarch64-linux arm riscv
4+
5+
//[x86_64] compile-flags: --target x86_64-unknown-uefi
6+
//[x86_64] needs-llvm-components: x86
7+
//[i686] compile-flags: --target i686-unknown-linux-musl
8+
//[i686] needs-llvm-components: x86
9+
//[aarch64-windows] compile-flags: --target aarch64-pc-windows-msvc
10+
//[aarch64-windows] needs-llvm-components: aarch64
11+
//[aarch64-linux] compile-flags: --target aarch64-unknown-linux-gnu
12+
//[aarch64-linux] needs-llvm-components: aarch64
13+
//[aarch64-apple] compile-flags: --target aarch64-apple-darwin
14+
//[aarch64-apple] needs-llvm-components: aarch64
15+
//[arm] compile-flags: --target armv7r-none-eabi
16+
//[arm] needs-llvm-components: arm
17+
//[riscv] compile-flags: --target riscv64gc-unknown-none-elf
18+
//[riscv] needs-llvm-components: riscv
19+
20+
// See bottom of file for a corresponding C source file that is meant to yield
21+
// equivalent declarations.
22+
#![feature(no_core, lang_items)]
23+
#![crate_type = "lib"]
24+
#![no_std]
25+
#![no_core]
26+
27+
#[lang="sized"] trait Sized { }
28+
#[lang="freeze"] trait Freeze { }
29+
#[lang="copy"] trait Copy { }
430

531
#[repr(i8)]
632
pub enum Type {
733
Type1 = 0,
834
Type2 = 1
935
}
1036

11-
// CHECK: define{{( dso_local)?}} noundef signext i8 @test()
37+
// To accommodate rust#97800, one might consider writing the below as:
38+
//
39+
// `define{{( dso_local)?}} noundef{{( signext)?}} i8 @test()`
40+
//
41+
// but based on rust#80556, it seems important to actually check for the
42+
// presence of the `signext` for those targets where we expect it.
43+
44+
// CHECK: define{{( dso_local)?}} noundef
45+
// x86_64-SAME: signext
46+
// aarch64-apple-SAME: signext
47+
// aarch64-windows-NOT: signext
48+
// aarch64-linux-NOT: signext
49+
// arm-SAME: signext
50+
// riscv-SAME: signext
51+
// CHECK-SAME: i8 @test()
52+
53+
1254
#[no_mangle]
1355
pub extern "C" fn test() -> Type {
1456
Type::Type1

‎src/test/codegen/pic-relocation-model.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ pub fn call_foreign_fn() -> u8 {
1010
}
1111
}
1212

13-
// CHECK: declare zeroext i8 @foreign_fn()
13+
// (Allow but do not require `zeroext` here, because it is not worth effort to
14+
// spell out which targets have it and which ones do not; see rust#97800.)
15+
16+
// CHECK: declare{{( zeroext)?}} i8 @foreign_fn()
1417
extern "C" {fn foreign_fn() -> u8;}
1518

1619
// CHECK: !{i32 {{[78]}}, !"PIC Level", i32 2}

0 commit comments

Comments
 (0)