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

Swapped Arguments in Generated Code for extern-C func on x64 #41375

Closed
Gankra opened this issue Apr 18, 2017 · 6 comments
Closed

Swapped Arguments in Generated Code for extern-C func on x64 #41375

Gankra opened this issue Apr 18, 2017 · 6 comments
Assignees

Comments

@Gankra
Copy link
Contributor

Gankra commented Apr 18, 2017

The following program has arg9 and arg10 swapped on the Rust side (C++ is passing the args correctly, as far as the AMD64 ABI is concerned).

Test Platform: macos x64

Reproduced On:

  • rustc 1.16.0 (30cf806 2017-03-10)
  • rustc 1.18.0-nightly (7627e3d 2017-04-16)

Reproduction Steps:

// lib.rs
pub struct BigArg1(u32, usize, usize, u32);
#[repr(C)] pub struct U32Arg2(u32);
#[repr(C)] pub struct TwoU32Arg5(u32, u32);
#[repr(C)] pub struct ComplexArg6 { size: usize, serialize_start_time: u64, serialize_end_time: u64 }
#[repr(C)] pub struct UsizeArg9 { size: usize }

#[no_mangle]
pub unsafe extern "C" fn wr_api_set_root_display_list(arg1: &mut BigArg1,
                                                      arg2: U32Arg2,
                                                      arg3: f32,
                                                      arg4: f32,
                                                      arg5: TwoU32Arg5,
                                                      arg6: ComplexArg6,
                                                      arg7: *mut u8,
                                                      arg8: usize,
                                                      arg9: UsizeArg9,
                                                      arg10: *mut u8,
                                                      arg11: usize) {

      println!("Arg01 (addr) {:?}", arg1 as *mut _);
      println!("Arg02 {:?}", arg2.0);
      println!("Arg03 {:?}", arg3);
      println!("Arg04 {:?}", arg4);
      println!("Arg05 {:?} {:?}", arg5.0, arg5.1);
      println!("Arg06 {:?} {:?} {:?}", arg6.size, arg6.serialize_start_time, arg6.serialize_end_time);
      println!("Arg07 {:?}", arg7);
      println!("Arg08 {:?}", arg8);
      println!("Arg09 {:?}", arg9.size);
      println!("Arg10 {:?}", arg10);
      println!("Arg11 {:?}", arg11);
}
// main.cpp

#include <stdlib.h>
#include <stdint.h>

struct BigArg1;
struct U32Arg2 { uint32_t a; };
struct TwoU32Arg5 { uint32_t a; uint32_t b; };
struct ComplexArg6 { size_t size; uint64_t serialize_start_time; uint64_t serialize_end_time; };
struct UsizeArg9 { size_t size; };

extern "C" {
  inline void
  wr_api_set_root_display_list(BigArg1* arg1,
      U32Arg2 arg2,
      float arg3,
      float arg4,
      TwoU32Arg5 arg5,
      ComplexArg6 arg6,
      uint8_t* arg7,
      size_t arg8,
      UsizeArg9 arg9,
      uint8_t* arg10,
      size_t arg11);
}


int main() {
  U32Arg2 arg2 = { 2 };
  TwoU32Arg5 arg5 = { 50, 51 };
  ComplexArg6 arg6 = { 60, 61, 62 };
  UsizeArg9 arg9 = { 9 };

  wr_api_set_root_display_list(
      (BigArg1*) 1,
      arg2,
      3.0,
      4.0,
      arg5,
      arg6,
      (uint8_t*) 7,
      8,
      arg9,
      (uint8_t*) 10,
      11
  );
}
rustc --crate-type=staticlib lib.rs
clang++ main.cpp liblib.a
./a.out

Arg01 (addr) 0x1
Arg02 2
Arg03 3
Arg04 4
Arg05 50 51
Arg06 60 61 62
Arg07 0x7
Arg08 8
Arg09 10
Arg10 0x9
Arg11 11
@eddyb
Copy link
Member

eddyb commented Apr 18, 2017

You might also need -lpthread -ldl to use clang++ with the Rust staticlib.

@eddyb
Copy link
Member

eddyb commented Apr 18, 2017

Looks like we use %UsizeArg9* byval noalias nocapture dereferenceable(8) (which would pass 8 bytes on the stack) while clang emits i64 for that same argument - at that point they should both go on the stack but I'm not sure what's happening, maybe LLVM really doesn't like what we're doing.

EDIT: adding one more usize after the 8th argument fixes it, so I think we're just placing the last register argument on the stack explicitly, whereas clang relies on LLVM making that decision.

@jrmuizel
Copy link
Contributor

Arg9 should be passed in r9. Since we expect it as %UsizeArg9* that's not happening.

@jrmuizel
Copy link
Contributor

EDIT: adding one more usize after the 8th argument fixes it, so I think we're just placing the last register argument on the stack explicitly, whereas clang relies on LLVM making that decision.

That matches my conclusion.

@eddyb
Copy link
Member

eddyb commented Apr 18, 2017

Looks like we're treating stack arguments as using up registers (not just the return pointer).
EDIT: it was wrong since it was added in #27017.

Minimal reproduction of the provided testcase (6 stack arguments, 1 register argument):

#![crate_type="staticlib"]
#[repr(C)] pub struct Big { data: [u8; 64] }
#[repr(C)] pub struct Small { data: u8 }

#[no_mangle]
pub extern "C" fn test(_: Big, _: Big, _: Big, _: Big, _: Big, _: Big, s: Small) {
    println!("{}", s.data);
}
typedef struct { unsigned char data[64]; } Big;
typedef struct { unsigned char data; } Small;

void test(Big, Big, Big, Big, Big, Big, Small);

int main() {
    Big big;
    Small small = { 42 };
    test(big, big, big, big, big, big, small);
}

@eddyb eddyb self-assigned this Apr 18, 2017
@eddyb
Copy link
Member

eddyb commented Apr 18, 2017

Testcase for Rust -> C call (roughly what will end up in run-make/extern-fn-struct-passing-abi):

#[repr(C)]
#[derive(Copy, Clone)]
struct Big { data: [i32; 5] }

#[repr(C)]
struct Small { data: i32 }

extern "C" {
    fn test(a: Big, b: Big, c: Big, d: Big, e: Big, f: Big, g: Small);
}

fn main() {
    let big = Big { data: [0; 5] };
    unsafe {
        test(big, big, big, big, big, big, Small { data: 42 });
    }
}
#include <assert.h>
#include <stdint.h>

typedef struct { int32_t data[5]; } Big;
typedef struct { int32_t data; } Small;

void test(Big a, Big b, Big c, Big d, Big e, Big f, Small g) {
    assert(g.data == 42);
}

frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
…elb1

rustc_trans: do not treat byval as using up registers.

Perhaps not that well-documented, `byval` pointer arguments *are not* the same as pointer arguments used by pass-by-ref, but rather the pointer is only used by LLVM to pass the *contents* on the stack.

Fixes rust-lang#41375.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
…elb1

rustc_trans: do not treat byval as using up registers.

Perhaps not that well-documented, `byval` pointer arguments *are not* the same as pointer arguments used by pass-by-ref, but rather the pointer is only used by LLVM to pass the *contents* on the stack.

Fixes rust-lang#41375.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 18, 2017
…elb1

rustc_trans: do not treat byval as using up registers.

Perhaps not that well-documented, `byval` pointer arguments *are not* the same as pointer arguments used by pass-by-ref, but rather the pointer is only used by LLVM to pass the *contents* on the stack.

Fixes rust-lang#41375.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 19, 2017
…elb1

rustc_trans: do not treat byval as using up registers.

Perhaps not that well-documented, `byval` pointer arguments *are not* the same as pointer arguments used by pass-by-ref, but rather the pointer is only used by LLVM to pass the *contents* on the stack.

Fixes rust-lang#41375.
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 20, 2017
…elb1

rustc_trans: do not treat byval as using up registers.

Perhaps not that well-documented, `byval` pointer arguments *are not* the same as pointer arguments used by pass-by-ref, but rather the pointer is only used by LLVM to pass the *contents* on the stack.

Fixes rust-lang#41375.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants