struct pass-by-value failing on SPARC

[dhduvall]

There are a handful of testsuite failures on SPARC that seem to be related, and they have to do with passing structs by value into C. Although in at least one case (extern-fn-struct-passing-abi), the C code isn't getting the right values (but there it's a floating-point value), in the integer case Shawn and I have dived into (extern-pass-TwoU8s), that part appears to be correct. It's the retrieval of the struct's return value that seems to be wrong.

The problem becomes a bit more apparent if we rebuild the test with -g instead of -O. I can attach full files if necessary, but here is (what I think is the relevant part of) the IR for the main() function:

; extern_pass_TwoU8s::main
; Function Attrs: uwtable
define internal void @_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE() unnamed_addr #0 !dbg !294 {
start:
  %tmp_ret1 = alloca %"core::fmt::ArgumentV1"
  %tmp_ret = alloca %"core::fmt::ArgumentV1"
  %abi_cast = alloca i16
  %arg = alloca %TwoU8s
  %__arg1 = alloca %TwoU8s**
  %__arg0 = alloca %TwoU8s**
  %_22 = alloca { %TwoU8s**, [0 x i8], %TwoU8s**, [0 x i8] }
  %_21 = alloca [2 x %"core::fmt::ArgumentV1"]
  %_16 = alloca %"core::fmt::Arguments"
  %right_val = alloca %TwoU8s*
  %left_val = alloca %TwoU8s*
  %_5 = alloca { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }
  %y = alloca %TwoU8s
  %x = alloca %TwoU8s
  call void @llvm.dbg.declare(metadata %TwoU8s* %x, metadata !297, metadata !144), !dbg !299
  call void @llvm.dbg.declare(metadata %TwoU8s* %y, metadata !300, metadata !144), !dbg !302
  call void @llvm.dbg.declare(metadata %TwoU8s** %left_val, metadata !303, metadata !144), !dbg !306
  call void @llvm.dbg.declare(metadata %TwoU8s** %right_val, metadata !307, metadata !144), !dbg !306
  call void @llvm.dbg.declare(metadata %TwoU8s*** %__arg0, metadata !308, metadata !144), !dbg !311
  call void @llvm.dbg.declare(metadata %TwoU8s*** %__arg1, metadata !312, metadata !144), !dbg !311
  %0 = getelementptr inbounds %TwoU8s, %TwoU8s* %x, i32 0, i32 0, !dbg !313
  store i8 22, i8* %0, !dbg !313
  %1 = getelementptr inbounds %TwoU8s, %TwoU8s* %x, i32 0, i32 2, !dbg !313
  store i8 23, i8* %1, !dbg !313
  %2 = getelementptr inbounds %TwoU8s, %TwoU8s* %x, i32 0, i32 0, !dbg !314
  %3 = getelementptr inbounds %TwoU8s, %TwoU8s* %x, i32 0, i32 2, !dbg !314
  %4 = load i8, i8* %2, !dbg !314
  %5 = load i8, i8* %3, !dbg !314
  %6 = getelementptr inbounds %TwoU8s, %TwoU8s* %arg, i32 0, i32 0, !dbg !314
  store i8 %4, i8* %6, !dbg !314
  %7 = getelementptr inbounds %TwoU8s, %TwoU8s* %arg, i32 0, i32 2, !dbg !314
  store i8 %5, i8* %7, !dbg !314
  %8 = bitcast %TwoU8s* %arg to i64*, !dbg !314
  %9 = load i64, i64* %8, align 1, !dbg !314
  %10 = call i16 @rust_dbg_extern_identity_TwoU8s(i64 %9), !dbg !314
  store i16 %10, i16* %abi_cast, !dbg !314
  %11 = bitcast %TwoU8s* %y to i8*, !dbg !314
  %12 = bitcast i16* %abi_cast to i8*, !dbg !314
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %11, i8* %12, i64 2, i32 1, i1 false), !dbg !314
  br label %bb1, !dbg !314

bb1:                                              ; preds = %start
  %13 = getelementptr inbounds { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }, { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }* %_5, i32 0, i32 0, !dbg !315
  store %TwoU8s* %x, %TwoU8s** %13, !dbg !315
  %14 = getelementptr inbounds { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }, { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }* %_5, i32 0, i32 2, !dbg !315
  store %TwoU8s* %y, %TwoU8s** %14, !dbg !315
  %15 = getelementptr inbounds { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }, { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }* %_5, i32 0, i32 0, !dbg !315
  %16 = load %TwoU8s*, %TwoU8s** %15, !dbg !315, !nonnull !91
  store %TwoU8s* %16, %TwoU8s** %left_val, !dbg !315
  %17 = getelementptr inbounds { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }, { %TwoU8s*, [0 x i8], %TwoU8s*, [0 x i8] }* %_5, i32 0, i32 2, !dbg !315
  %18 = load %TwoU8s*, %TwoU8s** %17, !dbg !315, !nonnull !91
  store %TwoU8s* %18, %TwoU8s** %right_val, !dbg !315
  %19 = load %TwoU8s*, %TwoU8s** %left_val, !dbg !306, !nonnull !91
  %20 = load %TwoU8s*, %TwoU8s** %right_val, !dbg !306, !nonnull !91
; call <extern_pass_TwoU8s::TwoU8s as core::cmp::PartialEq>::eq
  %21 = call zeroext i1 @"_ZN67_$LT$extern_pass_TwoU8s..TwoU8s$u20$as$u20$core..cmp..PartialEq$GT$2eq17h9059cf2eb03b4778E"(%TwoU8s* noalias readonly dereferenceable(2) %19, %TwoU8s* noalias readonly dereferenceable(2) %20), !dbg !306
  br label %bb2, !dbg !306

and the resulting assembly (more or less):

extern-pass-TwoU8s.stage2-sparcv> _ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE::dis
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE:       save      %sp, -0x1c0, %sp
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+4:     call      +0x8          <_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0xc>
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+8:     sethi     %hi(0x106c00), %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0xc:   or        %i0, 0x168, %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x10:  add       %i0, %o7, %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x14:  mov       0x16, %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x18:  stb       %i1, [%fp + 0x737]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x1c:  add       %fp, 0x737, %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x20:  or        %i1, 0x1, %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x24:  mov       0x17, %i2
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x28:  stb       %i2, [%i1]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x2c:  ldub      [%fp + 0x737], %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x30:  stb       %i1, [%fp + 0x7d7]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x34:  add       %fp, 0x7d7, %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x38:  or        %i1, 0x1, %i1
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x3c:  stb       %i2, [%i1]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x40:  ldx       [%fp + 0x7d7], %o0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x44:  call      +0x724        <rust_dbg_extern_identity_TwoU8s>
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x48:  stx       %i0, [%fp + 0x72f]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x4c:  sth       %o0, [%fp + 0x7dd]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x50:  lduh      [%fp + 0x7dd], %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x54:  ba        +0x8          <_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x5c>
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x58:  sth       %i0, [%fp + 0x73f]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x5c:  add       %fp, 0x737, %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x60:  stx       %i0, [%fp + 0x747]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x64:  add       %fp, 0x73f, %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x68:  stx       %i0, [%fp + 0x74f]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x6c:  ldx       [%fp + 0x747], %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x70:  stx       %i0, [%fp + 0x757]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x74:  ldx       [%fp + 0x74f], %i0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x78:  stx       %i0, [%fp + 0x75f]
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x7c:  ldx       [%fp + 0x757], %o0
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x80:  call      +0x240        <_ZN67_$LT$extern_pass_TwoU8s..TwoU8s$u20$as$u20$core..cmp..PartialEq$GT$2eq17h9059cf2eb03b4778E>
_ZN18extern_pass_TwoU8s4main17h7a27acd65aa2f60dE+0x84:  mov       %i0, %o1

We believe the sth instruction after the call to rust_dbg_extern_identity_TwoU8s is where it goes wrong, implying something wrong with the store instruction in the IR.

Here is the IR for the equivalent C function (albeit compiled by clang from a different version of LLVM):

; Function Attrs: nounwind
define signext i32 @main() #0 {
  %1 = alloca i32, align 4
  %t = alloca %struct.TwoU8s, align 1
  %u = alloca %struct.TwoU8s, align 1
  %2 = alloca i64, align 8
  %3 = alloca %struct.TwoU8s, align 1
  %4 = alloca i64, align 8
  store i32 0, i32* %1, align 4
  %5 = getelementptr inbounds %struct.TwoU8s, %struct.TwoU8s* %t, i32 0, i32 0
  store i8 22, i8* %5, align 1
  %6 = getelementptr inbounds %struct.TwoU8s, %struct.TwoU8s* %t, i32 0, i32 1
  store i8 23, i8* %6, align 1
  %7 = bitcast i64* %2 to i8*
  %8 = bitcast %struct.TwoU8s* %t to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %7, i8* %8, i64 2, i32 1, i1 false)
  %9 = load i64, i64* %2, align 8
  %10 = call i64 @rust_dbg_extern_identity_TwoU8s(i64 %9)
  store i64 %10, i64* %4, align 8
  %11 = bitcast i64* %4 to i8*
  %12 = bitcast %struct.TwoU8s* %3 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %12, i8* %11, i64 2, i32 1, i1 false)
  %13 = bitcast %struct.TwoU8s* %u to i8*
  %14 = bitcast %struct.TwoU8s* %3 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %13, i8* %14, i64 2, i32 1, i1 false)
  %15 = getelementptr inbounds %struct.TwoU8s, %struct.TwoU8s* %u, i32 0, i32 0
  %16 = load i8, i8* %15, align 1
  %17 = zext i8 %16 to i32
  %18 = icmp eq i32 %17, 22
  br i1 %18, label %21, label %19

Anyway, this was about as far as we got before we needed to take a break, but figured it was worth writing up and seeing if anyone here had any insights. This is all on the beta branch, at commit f38ffa8, though the failure happens on all versions of rust I've run the testsuite for.

@binarycrusader

[nagisa]

Similar failures have been observed at some point due to mismatching ABI expectations between the two sides (e.g. rustc still passes by copying value onto stack/registers, C side expects a pointer). That would be my first shot if I were to debug this.

[binarycrusader]

In this particular case, the C side is expecting a value, not a pointer, and the C side doesn't appear to be the problem.

That is, the value goes into the identity function and comes back out just as expected.

What seems to be wrong is the IR rust is generating for the rust side for accessing the return value.

A struct is passed into the C function and returned from the C function, but rust marks the function as returning an i16 instead, and when it goes to store the return value, it's attempting to store the lsb of the returned value instead of the msb.

[binarycrusader]

/cc @arielb1

[workingjubilee]

Requesting an update from someone who has a SPARC machine (preferably made of hard silicon, not QEMU, to be precise): We are 5 years out from this being opened. A lot has changed, in both LLVM and Rust. Does this issue persist? If yes, updates on the recent shape of recent errors would be great.

[dhduvall]

@binarycrusader and I have both moved on from Oracle. I don't know for sure about him, but I certainly don't have access to a SPARC machine anymore; I'm sorry. @alanc, I don't suppose there's any remaining internal need/support for Rust on SPARC, is there?

[alanc]

We still require Rust on SPARC in order to build Python Cryptography, GNOME, & Firefox for Solaris. @psumbera is the current maintainer of the Rust packages in Oracle Solaris.

[nagisa]

The GCC Farm Project has some SPARC machines and I have access to them. I’ll keep the tab open and might get to verifying this.

We had a some fixes to the SPARC ABI code recently, so chances that this might be fixed are quite significant.

[nagisa]

The extern-fn-struct-passing-abi test still fails with 1.59.0:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Huge { a: 5647, b: 5648, c: 5649, d: 5650, e: 0 }`,
 right: `Huge { a: 5647, b: 5648, c: 5649, d: 5650, e: 5651 }`', test.rs:125:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Unfortunately there are no nightly/beta/etc. builds of rustc for sparc, and I couldn’t be bothered enough to figure out how to crosscompile/get x.py work with the stable host rustc/if it would work at all.

1.59.0 sounds like a recent enough version to be a meaningful enough update for this issue though ^^

[nathaniel-bennett]

I'm getting this behavior on both SPARC and MIPS while trying to add struct definitions to the libc crate. When the architecture is 32-bit, it only affects the last field, and only for structs that aren't a multiple of 4 bytes (i.e. a u16 as the last field of a 14-byte struct); for 64-bit, it affects the last field, and only for structs that aren't a multiple of 8 bytes (such as a u32 as the last field of a 12-byte struct). The effect is that the last field is set to 0, at least for the few tests I've run.

Both the SPARC and MIPS architectures are big endian, and they were the only two big endian architectures tested for the libc build. Not sure if that's just coincidence, but it could potentially be linked to the cause of this bug.

These tests were ran and observed on qemu, not hard silicon.

[Jules-Bertholet]

@rustbot label I-unsound

[apiraino]

I feel this is related to #115609 and rust-lang/compiler-team#672

@rustbot label -I-prioritize