Issues/miscompilation around ARM T32 frame pointer with new asm syntax

[cbiffle]

Summary

First: the new asm! syntax reserves r11 on ARM as a frame pointer. This is incorrect on Thumb targets, where it should be r7. (llvm_asm! gets this right.)

Second: if you attempt to use r7, things miscompile. (This is true of llvm_asm! as well, and is the actual issue that brought me here.)

Code

I'm having a hard time writing a minimal reproduction, because you need to create a function with sufficient complexity to trigger reliance on the frame pointer. Here's what I've got.

Imagine that we have an OS syscall, invoked on ARM by the svc instruction, that needs to receive one parameter in r7. (This is simplified from our actual ABI, but we do use r7 for [reasons].)

asm!(
    "svc #0",
    in("r7") dest.as_mut_ptr(),
    options(nostack, readonly, preserves_flags)
);

We most often see this when a function (syscall stub) containing a block like that gets inlined into another function, but we've also encountered it within a single small function.

In the disassembly, we see stuff like this:

   2c492:       f1a7 003d       sub.w   r0, r7, #61      ; <-- here r7 is acting as frame pointer
   2c496:       9019            str     r0, [sp, #100]
   2c498:       a813            add     r0, sp, #76
  ; ... unrelated instructions, but which notably do not save r7 ...
   2c4ac:       460f            mov     r7, r1   ; <-- here, r7 is being loaded with the parameter
  ; ... unrelated instructions ...
   2c4b8:       df00            svc     0    ; <-- system call!
   2c4bc:       b11c            cbz     r4, 2c4c6
   ; ... unrelated instructions ...
   2c4e0:       f1a7 003d       sub.w   r0, r7, #61     ; <-- r7 is assumed to still be frame pointer
   2c4e4:       9019            str     r0, [sp, #100]

If one were to use r11 instead (which, spoiler, we also use), one would get a compiler error:

error: invalid register `r11`: the frame pointer cannot be used as an operand for inline asm
   --> userlib/src/lib.rs:191:13
    |
191 |             in("r11") Sysnum::BorrowRead as u32,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

...which is actually wrong for my target, but the intent is right: it appears to be trying to guard against this miscompilation case.

Expectations

I would normally expect that an explicit clobber would be enough for the compiler to not assume a particular value is live in a register across the asm! statement. This isn't true in either llvm_asm! or the new syntax. (I tried the new syntax to see if it fixed this -- and also because I expected a chilly reception to a bug in a deprecated syntax.)

Moreover, I argue that it is a misfeature to prevent the user from describing an asm sequence that damages the frame pointer. Such sequences can absolutely occur, particularly in cases of register bank switching, context restore, or naked functions that manage their own frames. The restriction on naming frame pointer should be loosened, in my opinion; the compiler can always generate code to save/restore it. (I'm currently ok with the ban on naming sp or pc.)

But barring that, the restriction should be correct.

Meta

This miscompilation was initially observed using llvm_asm! on the 2020-05-01 nightly. Confirmed with the new asm! syntax on:

rustc --version --verbose:

rustc 1.46.0-nightly (feb3536eb 2020-06-09)
binary: rustc
commit-hash: feb3536eba10c2e4585d066629598f03d5ddc7c6
commit-date: 2020-06-09
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0

The asm! announcement said I should add F-asm to this report, but it is not apparent how I would do that.

Hat tip to @labbott for identifying this.

[cbiffle]

In case anyone else is doing something really strange with registers and runs into this, the workaround is to not mention either of the registers that rustc doesn't like, and save/restore them in the asm. This implies a performance hit for register shuffling but avoids the miscompilation in my tests.

For example, a sequence that relies on taking inputs in both r7 and r11 (which, for better or worse, is a situation I have) might resemble this:

asm!("
    push {{r7, r11}}
    mov r7, {dest}
    mov r11, {sysnum}
    svc #0
    pop {{r7, r11}}
    ",
    dest = in(reg) dest.as_mut_ptr(),
    sysnum = const Sysnum::BorrowRead as u32,
);

Note that the push/pop means that you can't use options(nostack) anymore (not that, afaict, it has any effect on T32 targets). You could avoid use of the stack by swapping the offending registers through compiler-allocated temporaries:

asm!("
    mov {save7}, r7
    move {save11}, r11
    mov r7, {dest}
    mov r11, {sysnum}
    svc #0
    mov r7, {save7}
    move r11, {save11}
    ",
    dest = in(reg) dest.as_mut_ptr(),
    sysnum = const Sysnum::BorrowRead as u32,
    save7 = out(reg) _,
    save11 = out(reg) _,
    options(nostack),
);

Whether this is higher or lower cost than push/pop depends on your target system, RAM wait states, instruction prefetching behavior, multiple issue, etc.

[pitaj]

Given that this also happens with llvm_asm!, could this be an LLVM bug, or is it a rustc bug?

[cbiffle]

Not sure! I mostly use LLVM through rustc these days.

• The "using the wrong frame pointer for Thumb" is definitely a rustc bug, since that part is new with the new asm! syntax.
• The "not honoring clobbers properly" might well be an LLVM bug.
• The "preventing the user from expressing asm sequences that alter the frame pointer" is some combination of rustc and LLVM. (LLVM has some related code in it, by the look of things.)

[Amanieu]

"Not honoring clobbers properly" is definitely an LLVM bug. Sadly resolving it is non-trivial because LLVM uses the frame pointer to access the stack which is needed to save/restore register values.

It seems that LLVM 11 has added a check (ARM-only for now) on writing to reserved registers: https://reviews.llvm.org/D76848. However it would still be preferable to catch this in the front-end.

FYI here is the list of reserved registers on ARM:

• The pc and sp registers are obviously reserved.
• Either r7 or r11 is used as the frame pointer depending on the target.
• r6 is used as a "base pointer" in functions with large and/or "interesting" stack frames. I think we probably need to disallow this register in inline asm as well.
• On some targets r9 is also reserved (reserve-r9 target feature). None of our built-in target do so however.