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.

[cbiffle]

If LLVM isn't capable of saving and restoring a base pointer around an asm block with a conflicting clobber, then, yes, for correctness, we probably have to ban r6.

Having written some register allocators myself, I am disappointed with LLVM's behavior in these cases, but I imagine it simplifies things considerably for them.

Two questions though:

1. Is it possible to detect whether the current target is using reserve-r9 and conditionally prevent its explicit use? (I work on ARM targets that both do and do not use this feature; if I need explicit access to the thread/global-data pointer, which is usually what r9 is doing, I can just reference r9 by name in an asm block; on targets not reserving r9 it ought to be fair game.)
2. Is it possible to make the ban on frame pointer use conditional on whether the target is using a frame pointer? (I tend to disable it with a custom target spec for embedded use, because it significantly reduces code size.)

[Amanieu]

Is it possible to detect whether the current target is using reserve-r9 and conditionally prevent its explicit use?

Sure, you just need to add the reserve-r9 feature to src/librustc_codegen_llvm/llvm_util.rs and add a filter to the register in src/librustc_target/asm/arm.rs.

Is it possible to make the ban on frame pointer use conditional on whether the target is using a frame pointer?

Not really, "disabling" the frame pointer just tells the compiler that it doesn't have to use a frame pointer all the time, only when it is needed. A frame pointer is generally needed when you have dynamically-sized objects on the stack (alloca). We don't actually know whether a frame pointer is needed until late in the back-end after the stack layout for a function has been computed (in particular, this depends on inlining).

[cbiffle]

Understood, thank you!

[cbiffle]

The implications of preventing the user from talking about clobbering a register, when that register is in fact open to use by the register allocator, are subtle. (To me, at least.) For example, here is a lightly contrived syscall stub.

        asm!("
            mov {save11}, r11
            mov r11, {in11}
            svc #0
            mov r11, {save11}
            ",
            save11 = out(reg) _,
            in11 = in(reg) something_or_other,

            options(preserves_flags)
        );

Here, because the asm! macro won't let me declare a parameter in r11 (due to this very bug), I'm including instructions to shuffle values into their actual ABI-appointed locations within the asm sequence. And this results in a slightly different miscompilation, which has just bitten me this afternoon.

You see, in a complex enough function, LLVM eventually assigns the in11 or save11 operand to r11, which destroys the register saving code. I would like to be able to declare a hazard on r11 to prevent the register allocator from treating it as available for this purpose, but I can't -- the rules around mentioning framepointers prevent me from doing so.

This is a potential issue because the use of the framepointer/basepointer registers is unpredictable. My understanding of the current implementation is that it is not possible to express this sequence correctly using inline asm!, because I am forbidden from describing a clobber of the framepointer, but the compiler is not forbidden from assigning either save11 or in11 to the framepointer should it happen to not need it.

Is that correct? (Testing this assertion is difficult -- the miscompilation happens only in certain shapes of complex functions -- and I'm not yet familiar enough with the rustc-LLVM interface to check for myself.)

Or can we guarantee that the compiler won't allocate reg operands to reserved registers under any circumstances? (I think a guarantee of this strength is required to make this sort of code correct.)

[Amanieu]

My understanding of the current implementation is that it is not possible to express this sequence correctly using inline asm!,

You can push r11 to the stack and pop it afterwards, which avoids this issue.

Or can we guarantee that the compiler won't allocate reg operands to reserved registers under any circumstances? (I think a guarantee of this strength is required to make this sort of code correct.)

Unfortunately LLVM won't let us "reserve" r11 (by using it as dummy output) since that will trigger a backend error when r11 is used as frame pointer by the compiler. I don't think there is a way to solve this without modifying LLVM.