Skip to content

sparc ABI issue when C program calls Rust function which returns structure #52638

Description

@psumbera

This was original encountered with Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1476252

Following is sample program which builds on https://github.com/alexcrichton/rust-ffi-examples

The issue is that on Sparc (tested on Solaris) the returing structure contains just zeros (on intel Solaris it works as expected).

git clone https://github.com/alexcrichton/rust-ffi-examples.git
cd rust-ffi-examples
gpatch -p1 <<EOF
index 78b6a46..0a4f23b 100644
--- a/cpp-to-rust/src/example.h
+++ b/cpp-to-rust/src/example.h
@@ -7,6 +7,12 @@ extern "C"{

 int double_input(int input);

+struct MyStruct {
+  bool a;
+};
+
+const MyStruct GetMyStruct();
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/cpp-to-rust/src/lib.rs b/cpp-to-rust/src/lib.rs
index bf573d1..96270c2 100644
--- a/cpp-to-rust/src/lib.rs
+++ b/cpp-to-rust/src/lib.rs
@@ -2,3 +2,16 @@
 pub extern fn double_input(input: i32) -> i32 {
     input * 2
 }
+
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct MyStruct {
+    pub a: bool,
+}
+
+#[no_mangle]
+pub extern fn GetMyStruct() -> MyStruct {
+    MyStruct {
+        a: true,
+    }
+}
diff --git a/cpp-to-rust/src/main.cpp b/cpp-to-rust/src/main.cpp
index e4c5ced..f66c298 100644
--- a/cpp-to-rust/src/main.cpp
+++ b/cpp-to-rust/src/main.cpp
@@ -7,6 +7,8 @@ int main() {
   int input = 10;
   int output = double_input(input);
   cout<<input<<" * 2 = "<<output<<"\n";
+  const MyStruct mystruct = GetMyStruct();
+  cout<<"GetMyStruct returns "<<mystruct.a<<"\n";

   return 0;
 }
EOF
cd cpp-to-rust
gmake
10 * 2 = 20
GetMyStruct returns 0  <==== Should be 1!!!

Activity

  1. psumbera commented on Jul 23, 2018

    @psumbera
    ContributorAuthor

    @glaubitz @eddyb @jrtc27 this is similar to #46679 Any comment on this?

  2. glaubitz commented on Jul 23, 2018

    @glaubitz
    Contributor

    Hmm, interesting. @jrtc27 has more in-depth knowledge regarding the ABI though, so I won't be of much help here.

    I can test though whether this issue shows on linux-sparc64 as well.

    @psumbera Btw, does incremental compilation work for you on solaris-sparc64?

  3. jrtc27 commented on Jul 23, 2018

    @jrtc27

    On 32-bit SPARC structs are always returned indirectly (extra pointer passed to the function at the start of the parameter array, although on 32-bit SPARC this is always a reserved slot) and will set %o0/%i0 (depending on your point of view) to be this pointer when returning, whereas 64-bit SPARC will returns small structs in registers. For which are you compiling, and what's the disassembly of GetMyStruct? GCC 7.3.0 gives the following for the C equivalent:

    GetMyStruct:                  ; sparc32-linux-gnu
    	ld	[%sp+64], %g1 ; Load return value pointer
    	mov	1, %g2        ; Put true in register
    	mov	%g1, %o0      ; Put pointer in %o0 for caller
    	jmp	%o7+12        ; Return (skipping over unimp)
    	 stb	%g2, [%g1]    ; Write true to struct (delay slot)
    GetMyStruct:                 ; sparc64-linux-gnu
    	mov	1, %o0       ; Put true in register
    	jmp	%o7+8        ; Return (no unimp on sparc64)
    	 sllx	%o0, 56, %o0 ; Move true to top byte (delay slot)

    Note that 32-bit SPARC has an immediate of 12 for its jump (i.e. skipping over 3 instructions, not just the call/delay slot pair), as a call site which is expecting a struct return value includes an unimp instruction directly after the call.

  4. psumbera commented on Jul 23, 2018

    @psumbera
    ContributorAuthor

    I'm building 64bit code (Rust on Solaris supports just 64bits).

    GetMyStruct:                    save      %sp, -0x90, %sp
    GetMyStruct+4:                  mov       0x1, %i0
    GetMyStruct+8:                  stb       %i0, [%fp + 0x7f7]
    GetMyStruct+0xc:                ret
    GetMyStruct+0x10:               restore   %g0, 0x1, %o0
    
  5. psumbera commented on Jul 23, 2018

    @psumbera
    ContributorAuthor

    Where g++ compiles following:

    #include <iostream>
    
    using namespace std;
    
    struct MyStruct {
      bool a;
    };
    
    MyStruct GetMyStruct()
    {
      MyStruct a;
      a.a=true;
      return a;
    }
    
    int main()
    {
      const MyStruct mystruct = GetMyStruct();
      cout<<mystruct.a<<"\n";
      return 0;
    }
    

    into:

    _Z11GetMyStructv:               save      %sp, -0xc0, %sp
    _Z11GetMyStructv+4:             mov       0x1, %g1
    _Z11GetMyStructv+8:             stb       %g1, [%fp + 0x7fe]
    _Z11GetMyStructv+0xc:           ldub      [%fp + 0x7fe], %g1
    _Z11GetMyStructv+0x10:          and       %g1, 0xff, %g1
    _Z11GetMyStructv+0x14:          sllx      %g1, 0x38, %g1
    _Z11GetMyStructv+0x18:          mov       %g1, %i0
    _Z11GetMyStructv+0x1c:          return    %i7 + 0x8
    _Z11GetMyStructv+0x20:          nop
    
    
  6. jrtc27 commented on Jul 23, 2018

    @jrtc27

    I don't know why it's spilling %i0 to the first stack frame slot (probably some LLVM stupidity), but you can see the final instruction in the delay slot is what does the small struct return value. However, it's doing %o0 = %g0 + 0x1 (whilst also restoring the register window), which puts the 1 in the low byte of %o0 rather than the high byte as required (mirroring the endianness of the struct in memory). Given that Clang doesn't do this (it generates extremely stupid code, but it's at least correct), I suspect the problem is https://github.com/rust-lang/rust/blob/master/src/librustc_target/abi/call/sparc64.rs#L70-L73 as that's just doing a cast rather than telling LLVM it needs to do a shift.

  7. psumbera commented on Jul 24, 2018

    @psumbera
    ContributorAuthor

    @jrtc27 I think I understand now need to shift. Is there any similar code I could leverage? I'm still rust beginner.. Thanks!

  8. psumbera commented on Jul 25, 2018

    @psumbera
    ContributorAuthor

    I'm experimenting with following and few my test cases work with it:

    --- rustc-1.26.2-src/src/librustc_trans/cabi_sparc64.rs
    +++ rustc-1.26.2-src/src/librustc_trans/cabi_sparc64.rs
    @@ -51,16 +51,7 @@
         let size = ret.layout.size;
         let bits = size.bits();
         if bits <= 256 {
    -        let unit = if bits <= 8 {
    -            Reg::i8()
    -        } else if bits <= 16 {
    -            Reg::i16()
    -        } else if bits <= 32 {
    -            Reg::i32()
    -        } else {
    -            Reg::i64()
    -        };
    -
    +        let unit = Reg::i64();
             ret.cast_to(Uniform {
                 unit,
                 total: size		 
    
  9. added
    O-SPARCTarget: SPARC processors
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    A-FFIArea: Foreign function interface (FFI)
    on Jul 25, 2018
  10. nagisa commented on Jul 25, 2018

    @nagisa
    Member

    Is there a normative document on SPARC ABI anywhere?

    cc @eddyb (as you fiddle around the cabi a lot)

  11. jrtc27 commented on Jul 25, 2018

    @jrtc27

    Yes, the SPARC Compliance Definition; for 64-bit, the latest is SCD 2.4.1 (2.3 for 32-bit). They're available from http://sparc.org/technical-documents/.

  12. nagisa commented on Jul 25, 2018

    @nagisa
    Member

    (EDIT: ignore whatever stupid stuff I said here, this stupid stuff has been removed since)

    Test code
    #![feature(no_core, lang_items)]
    #![no_core]
    
    #[lang="sized"]
    trait Sized { }
    #[lang="freeze"]
    trait Freeze { }
    #[lang="copy"]
    trait Copy { }
    
    #[repr(C)]
    pub struct Bool {
        b: bool,
    }
    
    pub fn plainbool() -> bool {
        true
    }
    
    
    pub fn structbool() -> Bool {
        Bool { b: true }
    }

    when compiled as such: rustc --target=sparc64-unknown-linux-gnu --emit=llvm-ir,asm --crate-type=rlib test.rs -O results in following llvm-ir:

    rustc llvm-ir
    define i8 @_ZN4test10structbool17he551889a1dd379c7E() unnamed_addr #0 {
    start:
      ret i8 1
    }
    

    as opposed to the following IR as generated by clang:

    clang llvm-ir
    define i64 @GetMyStruct() local_unnamed_addr #0 {
      ret i64 72057594037927936
    }
    

    This is extremely weird behaviour, and will probably be fairly difficult to implement.


    I’m honestly fairly surprised that the patch above does work.

  13. nagisa commented on Jul 25, 2018

    @nagisa
    Member

    @psumbera The patch above is fine. Do you want to submit a PR with it?

  14. added a commit that references this issue on Jul 26, 2018
    995d719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-FFIArea: Foreign function interface (FFI)O-SPARCTarget: SPARC processorsT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions