Skip to content

Commit

Permalink
Rollup merge of rust-lang#62380 - eddyb:x64-sysv-regs, r=nagisa
Browse files Browse the repository at this point in the history
rustc_target: avoid negative register counts in the SysV x86_64 ABI.

Because `needed_{int,sse}` and `{int,sse}_regs` were only used with integer literals, they were inferred to `i32` and `{int,sse}_regs` could therefore be negative.
There was a check which prevented that, but *only* for aggregate arguments, not scalars.

Fixes rust-lang#62350.

r? @nagisa or @rkruppe
  • Loading branch information
Centril committed Jul 5, 2019
2 parents 07505c9 + 050a71b commit b973796
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 9 deletions.
33 changes: 24 additions & 9 deletions src/librustc_target/abi/call/x86_64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,29 +167,44 @@ fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
target
}

const MAX_INT_REGS: usize = 6; // RDI, RSI, RDX, RCX, R8, R9
const MAX_SSE_REGS: usize = 8; // XMM0-7

pub fn compute_abi_info<'a, Ty, C>(cx: &C, fty: &mut FnType<'a, Ty>)
where Ty: TyLayoutMethods<'a, C> + Copy,
C: LayoutOf<Ty = Ty, TyLayout = TyLayout<'a, Ty>> + HasDataLayout
{
let mut int_regs = 6; // RDI, RSI, RDX, RCX, R8, R9
let mut sse_regs = 8; // XMM0-7
let mut int_regs = MAX_INT_REGS;
let mut sse_regs = MAX_SSE_REGS;

let mut x86_64_ty = |arg: &mut ArgType<'a, Ty>, is_arg: bool| {
let mut cls_or_mem = classify_arg(cx, arg);

let mut needed_int = 0;
let mut needed_sse = 0;
if is_arg {
if let Ok(cls) = cls_or_mem {
let mut needed_int = 0;
let mut needed_sse = 0;
for &c in &cls {
match c {
Some(Class::Int) => needed_int += 1,
Some(Class::Sse) => needed_sse += 1,
_ => {}
}
}
if arg.layout.is_aggregate() && (int_regs < needed_int || sse_regs < needed_sse) {
cls_or_mem = Err(Memory);
match (int_regs.checked_sub(needed_int), sse_regs.checked_sub(needed_sse)) {
(Some(left_int), Some(left_sse)) => {
int_regs = left_int;
sse_regs = left_sse;
}
_ => {
// Not enough registers for this argument, so it will be
// passed on the stack, but we only mark aggregates
// explicitly as indirect `byval` arguments, as LLVM will
// automatically put immediates on the stack itself.
if arg.layout.is_aggregate() {
cls_or_mem = Err(Memory);
}
}
}
}
}
Expand All @@ -201,14 +216,14 @@ pub fn compute_abi_info<'a, Ty, C>(cx: &C, fty: &mut FnType<'a, Ty>)
} else {
// `sret` parameter thus one less integer register available
arg.make_indirect();
// NOTE(eddyb) return is handled first, so no registers
// should've been used yet.
assert_eq!(int_regs, MAX_INT_REGS);
int_regs -= 1;
}
}
Ok(ref cls) => {
// split into sized chunks passed individually
int_regs -= needed_int;
sse_regs -= needed_sse;

if arg.layout.is_aggregate() {
let size = arg.layout.size;
arg.cast_to(cast_target(cls, size))
Expand Down
23 changes: 23 additions & 0 deletions src/test/auxiliary/rust_test_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,29 @@ uint64_t get_c_many_params(void *a, void *b, void *c, void *d, struct quad f) {
return f.c;
}

struct quad_floats {
float a;
float b;
float c;
float d;
};

float get_c_exhaust_sysv64_ints(
void *a,
void *b,
void *c,
void *d,
void *e,
void *f,
// `f` used the last integer register, so `g` goes on the stack.
// It also used to bring the "count of available integer registers" down to
// `-1` which broke the next SSE-only aggregate argument (`h`) - see #62350.
void *g,
struct quad_floats h
) {
return h.c;
}

// Calculates the average of `(x + y) / n` where x: i64, y: f64. There must be exactly n pairs
// passed as variadic arguments. There are two versions of this function: the
// variadic one, and the one that takes a `va_list`.
Expand Down
36 changes: 36 additions & 0 deletions src/test/run-pass/abi-sysv64-arg-passing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// extern-return-TwoU64s
// foreign-fn-with-byval
// issue-28676
// issue-62350-sysv-neg-reg-counts
// struct-return

// ignore-android
Expand Down Expand Up @@ -83,6 +84,9 @@ mod tests {
#[derive(Copy, Clone)]
pub struct Quad { a: u64, b: u64, c: u64, d: u64 }

#[derive(Copy, Clone)]
pub struct QuadFloats { a: f32, b: f32, c: f32, d: f32 }

#[repr(C)]
#[derive(Copy, Clone)]
pub struct Floats { a: f64, b: u8, c: f64 }
Expand All @@ -108,6 +112,16 @@ mod tests {
pub fn get_z(x: S) -> u64;
pub fn get_c_many_params(_: *const (), _: *const (),
_: *const (), _: *const (), f: Quad) -> u64;
pub fn get_c_exhaust_sysv64_ints(
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
h: QuadFloats,
) -> f32;
pub fn rust_dbg_abi_1(q: Quad) -> Quad;
pub fn rust_dbg_abi_2(f: Floats) -> Floats;
}
Expand Down Expand Up @@ -263,6 +277,27 @@ mod tests {
test();
}

fn test_62350() {
use std::ptr;
unsafe {
let null = ptr::null();
let q = QuadFloats {
a: 10.2,
b: 20.3,
c: 30.4,
d: 40.5
};
assert_eq!(
get_c_exhaust_sysv64_ints(null, null, null, null, null, null, null, q),
q.c,
);
}
}

pub fn issue_62350() {
test_62350();
}

fn test1() {
unsafe {
let q = Quad { a: 0xaaaa_aaaa_aaaa_aaaa,
Expand Down Expand Up @@ -321,6 +356,7 @@ fn main() {
extern_return_twou64s();
foreign_fn_with_byval();
issue_28676();
issue_62350();
struct_return();
}

Expand Down
46 changes: 46 additions & 0 deletions src/test/run-pass/abi/issues/issue-62350-sysv-neg-reg-counts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// run-pass
#![allow(dead_code)]
#![allow(improper_ctypes)]

// ignore-wasm32-bare no libc to test ffi with

#[derive(Copy, Clone)]
pub struct QuadFloats { a: f32, b: f32, c: f32, d: f32 }

mod rustrt {
use super::QuadFloats;

#[link(name = "rust_test_helpers", kind = "static")]
extern {
pub fn get_c_exhaust_sysv64_ints(
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
_: *const (),
h: QuadFloats,
) -> f32;
}
}

fn test() {
unsafe {
let null = std::ptr::null();
let q = QuadFloats {
a: 10.2,
b: 20.3,
c: 30.4,
d: 40.5
};
assert_eq!(
rustrt::get_c_exhaust_sysv64_ints(null, null, null, null, null, null, null, q),
q.c,
);
}
}

pub fn main() {
test();
}

0 comments on commit b973796

Please sign in to comment.