ARM ABI Violation on extern function that returns u8

[fpgaminer]

Rust will violate the ARM ABI on functions like pub extern "C" fn bsp_callback() -> u8 by not zero extending into r0. This causes problems for C code that calls those functions.

Reduced reproduction code/project available at this gist:
https://gist.github.com/fpgaminer/88e26978e023652fb1dd

Replace ">" in the file names with "/" to restore file structure. Compile C library "foo" by running make in the foo directory. Compile project using cargo build --target=thumbv6m-none-eabi --release. Disassembly included in the Gist.

In the bsp_callback function's disassembly I expected to see this:

ldrb r0, [...]    // Load our test byte
strb r0, [...]    // Store temporarily on the stack
...   // other functionality
ldrb r0, [...]    // Load our test byte from the stack
pop {..., pc}   // Return

Instead, this happened:

080000d0 <bsp_callback>:
 80000d0:   b5f0        push    {r4, r5, r6, r7, lr}
 80000d2:   b08d        sub sp, #52 ; 0x34
 80000d4:   481b        ldr r0, [pc, #108]  ; (8000144 <bsp_callback+0x74>)
 80000d6:   7800        ldrb    r0, [r0, #0]
 80000d8:   a904        add r1, sp, #16
 80000da:   7008        strb    r0, [r1, #0]
...   // other functionality
 800013a:   9804        ldr r0, [sp, #16]
 800013c:   b00d        add sp, #52 ; 0x34
 800013e:   bdf0        pop {r4, r5, r6, r7, pc}

Notice the use of ldr r0, [sp, #16] before the return. This results in the upper 3 bytes (on a 32-bit system) of r0 being loaded with garbage. The ARM ABI states "A Fundamental Data Type that is smaller than 4 bytes is zero- or sign-extended to a word and returned in r0." So Rust is violating the ABI here. The C code that calls bsp_callback depends on this ABI and will fail due to the garbage bytes.

Sorry for the large amount of code in the test project. I encountered this bug in a much larger project, and quickly reduced it down to a minimal test case. It's hard to get a test case that properly clobbers the bytes that ldr loads, plus this project targets ARM so that adds a whole mess of extra code. bsp_callback here just does some raw pointer dereferencing to quickly get data that won't be optimized away. print! and semihosting are also used to prevent optimizing away code, and to give the compiler a chance to pour garbage into the stack so the bug is obvious.

This reproduction case requires compiling in release mode, but the project where I encounter the bug was in debug mode.

Meta

rustc 1.8.0-nightly (38e23e8 2016-01-27)
binary: rustc
commit-hash: 38e23e8
commit-date: 2016-01-27
host: x86_64-unknown-linux-gnu
release: 1.8.0-nightly

arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors) 5.2.1 20151202 (release) [ARM/embedded-5-branch revision 231848]

ARMv6, Cortex-M0 (STM32F072)

Too Long; Didn't Reproduce

For the lazy: Go to the Gist (https://gist.github.com/fpgaminer/88e26978e023652fb1dd), check the disassembly for bsp_callback which was declared pub extern "C" fn bsp_callback() -> u8. Notice that the assembly uses strb to save r0, and then later uses ldr to load it right before returning. This leaves r0 with garbage in its upper bytes. The ARM ABI says the upper bytes should be 0 in this case.

[MagaTailor]

This is a custom llvm target - could it be an upstream bug?
(I take it you can vouch for the validity of the json target)

[Aatch]

Hmm, looks like the same rough issue as #30841. The issue only really comes up when using FFI, as the signature for bsp_callback in Rust looks like this:

i8 bsp_callback();

Whereas in C it looks like this:

i8 zeroext bsp_callback();

(Using LLVM IR syntax)

Rust code calling bsp_callback doesn't expect the return to be zero-extended, so it will work around it itself, while the C code is expecting the return to be extended and therefore doesn't work around it.

There is a workaround in the meantime, use u32 as the return type in Rust instead of u8, that should cause it work correctly.

[sanxiyn]

This seems fixed by #32732.

[fpgaminer]

Yup, looks like it. I recompiled my testcase and the disassembly shows the use of ldrb instead of the previously incorrect ldr. I haven't tested on live hardware, but the disassembly should be correct.