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
This commit is contained in:
bors 2022-09-16 20:08:05 +00:00
commit 95a992a686
11 changed files with 423 additions and 11 deletions

View File

@ -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<Uniform>
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 fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}
classify_arg(cx, arg);
classify_arg(cx, arg, param_policy);
}
}

View File

@ -685,7 +685,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),

View File

@ -1352,6 +1352,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,

View File

@ -1,6 +1,7 @@
// Helper functions used only in tests
#include <stdint.h>
#include <stdlib.h>
#include <assert.h>
#include <stdarg.h>
@ -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 */
}

View File

@ -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

View File

@ -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 {{[78]}}, !"PIC Level", i32 2}

View File

@ -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 <stdlib.h>
#include <stdint.h>
#include <stdio.h>
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; }
"##;

View File

@ -0,0 +1,14 @@
-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
# `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)

View File

@ -0,0 +1,24 @@
#include <stdlib.h>
#include <stdint.h>
#include <stdio.h>
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 */
}

View File

@ -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 pnkfelix'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_));
}

View File

@ -0,0 +1,39 @@
// run-pass
// ignore-wasm
#![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 pnkfelix'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_));
}