Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aarch64 call abi does not zeroext (and one cannot assume it does so) #97800

Merged
Prev Previous commit
Next Next commit
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.
  • Loading branch information
pnkfelix committed Jul 6, 2022
commit b2777aba757dd92042932d79309132bd953e130d
12 changes: 12 additions & 0 deletions src/test/run-make-fulldeps/issue-97463-abi-param-passing/Makefile
Original file line number Diff line number Diff line change
@@ -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)
24 changes: 24 additions & 0 deletions src/test/run-make-fulldeps/issue-97463-abi-param-passing/bad.c
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Member

@nagisa nagisa Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test setup for ABI tests with auxiliary/rust_test_helpers.c and ui/abi. Can that mechanism be used instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me go inspect that.

(whatever it is, it will need to use -O to replicate the problems here...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auxiliary/rust_test_helpers.c doesn't currently include <stdlib.h>, which is "needed" here if we want to reproduce the original test faithfully, in that it calls malloc. (And given how sensitive reproducing the failure is to small changes, I'm inclined to assume that malloc is a necessity.)

@nagisa do you know offhand if we are deliberately trying to avoid introducing a dependence from rust_test_helpers.c upon stdlib.h and/or malloc itself, perhaps to ensure we can compile the file on esoteric targets that do not have that available? Just curious.

I'll keep moving forward with trying to move this test into auxiliary/rust_test_helpers.c, under the assumption that we can add such a dependence. But given the amount of work I put into getting the codegen tests working, I'm sort of inclined to just live with a run-make test here, if I encounter any hurdles on the way...

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 */
}
Original file line number Diff line number Diff line change
@@ -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.
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
// (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_));
}