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

[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!!!

[psumbera]

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

[glaubitz]

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?

[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.

[psumbera]

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

[psumbera]

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

[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.

[psumbera]

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

[psumbera]

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		 

[nagisa]

Is there a normative document on SPARC ABI anywhere?

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

[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/.

[nagisa]

(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.

[nagisa]

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