[ARM] armv6m eXecute Only (XO) long branch Thunk

[keith.walker.arm]

Summary

This patch adds a thunk for Thumb long branch on V6-M for eXecute Only.

Note that there is currently no support for a position independent and
eXecute Only V6-M long branch thunk.

[stuij]

lld/ELF/Thunks.cpp

774

Not sure if we should prefer speed over space, but seeing the speed penalty, wouldn't it be better to use r12 instead of the stack to store the address?

[peter.smith]

Can you put [LLD] in the title [LLD][ARM] armv6m eXecute Only (XO) long branch Thunk?

Note that there is currently no support for a position independant

Possible Typo, I think it is position independent.

I think the use case of position independence and execute-only on v6-m, and requirements for long branches will be exceedingly rare so I think an error message is good enough. I think a PI variant is possible in theory although I don't think the existing ABI relocations could be used, would need to manually code them or alter the ABI.

lld/ELF/Thunks.cpp

780

Did you mean R_ARM_THM_ALU_ABS_G0_NC?

lld/test/ELF/arm-thumb-thunk-v6m-xo.s

4

New tests are being written with split-file which makes it easier to edit the linker script. For an example see aarch64-long-thunk-converge.s

For example:
// --- a.lds
SECTIONS {
.text_low : { *(.text_low) }
.text_high 0x2000000 : { *(.text_high) }
}

// --- a.s
.syntax unified

5

I think it would be better to start with a known constant value as then you can make the distance just out of range.

6

There is no .text_low2 and .text_high2 in this example, they can be removed.

7

The destination has a lot of zeros in it. Possibly worth choosing a number that will show up more non-zero identical immediates ? For example 0x21345678 (which I think is 2-byte aligned).

[peter.smith]

lld/ELF/Thunks.cpp

774

I think that would be

push {r0}
...
mov r12, r0
pop {r0}
bx r12

Looking at the M0 cycle counts

push {r0, r1} is 1 more cycle that push {r0} (1 + N) cycles where N is number of registers
mov r12, r0 is 1 cycle
pop {r0, pc} is 4 cycles more than pop {ro} (4 + N) with branch, (1 + N) without.
bx r12 is 3 cycles.

If I've got that correct the alternative sequence using r12 is 1 fewer cycle for 4-bytes extra code-size. For M0 I would tend to go for space over speed for 1 cycle.

[keith.walker.arm]

Updated to address review comments:

• Decision is to make no change to the thunk code sequence. As per Peters calculations the difference is 4 bytes of code vs 1 less instruction cycle. Decided that the 4 bytes codes saving was more important.
• Corrected the usage of R_ARM_THM_ALU_ABS_G0_NC.
• Rewrote the test to use the split-file feature
• Changed addresses of the test sections to :
  • Have unique values for each applied relocation.
  • Have the 2 sections near the limit of when it is required (see below)
• Removed unused section specification from linker script.

During testing I found that there is an issue if the distance between the source & destination is exactly 16M. A branch instruction is generated (rather than the long thunk code squence) which is not legal for armv6-m which only allows branches +/-2K. However this is not specific to this XO change, it already happens if XO is not involved so I think this is something that should be fixed separately.
The current test has an offset of 16M-4

[keith.walker.arm]

lld/ELF/Thunks.cpp

774

Agrred with Peter's reasoning and making no change.

[keith.walker.arm]

Minor correction to a previous stement: The current test has an offset of 16M+4

[peter.smith]

During testing I found that there is an issue if the distance between the source & destination is exactly 16M. A branch instruction is generated (rather than the long thunk code squence) which is not legal for armv6-m which only allows branches +/-2K. However this is not specific to this XO change, it already happens if XO is not involved so I think this is something that should be fixed separately.
The current test has an offset of 16M-4

I had thought something similar, but then managed to convince myself it wouldn't happen as LLD wouldn't generate a thunk for the narrow branch relocation, forgetting that these could be generated from BL relocations. I agree that it would be best fixed as a separate patch rather than tagged on to this one.

Thanks for the update. I've only got a few more nits that I should have noticed first time round. Otherwise this looks good to me.

lld/ELF/Thunks.cpp

1331

LLD tends not to use auto unless unless it is the result of a cast or an iterator. I think const bool would be most idiomatic.

lld/test/ELF/arm-thumb-thunk-v6m-xo.s

5

can you add --no-show-raw-insn as this will remove the raw encoding from the test.

43

Can you remove the addresses from the disassembly? We usually leave these off unless they are significant to the test as it makes the test more fragile and harder to update if there are minor changes.

[keith.walker.arm]

Made the following changes:

• Added --no-show-raw-insn to the test and removed the raw instructions from the expected test cases
• removed addresss from expected test cases, expect for the 1st line in each section to ensure that the section start address is correct.
• replaced used of auto with explicit type,

[MaskRay]

In D153772#4452896, @peter.smith wrote:

During testing I found that there is an issue if the distance between the source & destination is exactly 16M. A branch instruction is generated (rather than the long thunk code squence) which is not legal for armv6-m which only allows branches +/-2K. However this is not specific to this XO change, it already happens if XO is not involved so I think this is something that should be fixed separately.
The current test has an offset of 16M-4

I had thought something similar, but then managed to convince myself it wouldn't happen as LLD wouldn't generate a thunk for the narrow branch relocation, forgetting that these could be generated from BL relocations. I agree that it would be best fixed as a separate patch rather than tagged on to this one.

Thanks for the update. I've only got a few more nits that I should have noticed first time round. Otherwise this looks good to me.

Ideally, we should improve the test to test the limit of the range extension thunk. ppc64-long-branch-pi.s checks this but such tests are not common for range extension thunk support.
But I'm ok if this is too difficult to create.

I'll be out of town til July 5. I think the code is good. Accepted once @peter.smith is happy as well.

lld/ELF/Thunks.cpp

1340

place single quotes around the symbol name.

lld/test/ELF/arm-thumb-thunk-v6m-xo.s

7

For small output files, we don't typically remove them. Delete // RUN: rm -f %t/a %t/a2

10

New tests use ## for non-RUN-non-CHECK comments. For Arm where the universal comment marker // is preferred, use ///.

30

We can add another bl far to demonstrate that the thunk can be shared.

43

See my comment https://reviews.llvm.org/D153264#inline-1489080 for a prevailing style in newer tests.

// CHECK: Disassembly of section .text_low: can probably be removed.

[peter.smith]

LGTM too, assuming MaskRays comments can be addressed prior to commit.

[keith.walker.arm]

In D153772#4457576, @MaskRay wrote:

Ideally, we should improve the test to test the limit of the range extension thunk. ppc64-long-branch-pi.s checks this but such tests are not common for range extension thunk support.
But I'm ok if this is too difficult to create.

I did originally try to add a range limit check and that was when I found (and mentioned in an earlier comment) that for armv6-m in general (not just XO) a branch instuction is generated which is not supported by the armv6-m architecure. Fixing that issue is outside the scope of this change specifically aimed at XO. Attempting to add a range limit check at this time would enshrine the invalid branch instruction as being correct. I think adding the range limit checks should be added when this branch problem is fixed for armv6-m in general.

lld/ELF/Thunks.cpp

public:
   ThumbV6MABSLongThunk(Symbol &dest, int64_t addend)
       : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 12; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
+class ThumbV6MABSXOLongThunk final : public ThumbThunk {
+public:
+  ThumbV6MABSXOLongThunk(Symbol &dest, int64_t addend)
+      : ThumbThunk(dest, addend) {}
+
+  uint32_t sizeLong() override { return 20; }
+  void writeLong(uint8_t *buf) override;
+  void addSymbols(ThunkSection &isec) override;
+};
+
 class ThumbV6MPILongThunk final : public ThumbThunk {
 public:
   ThumbV6MPILongThunk(Symbol &dest, int64_t addend)
       : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 16; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;

 void ThumbV6MABSLongThunk::addSymbols(ThunkSection &isec) {
   addSymbol(saver().save("__Thumbv6MABSLongThunk_" + destination.getName()),
             STT_FUNC, 1, isec);
   addSymbol("$t", STT_NOTYPE, 0, isec);
   if (!getMayUseShortThunk())
     addSymbol("$d", STT_NOTYPE, 8, isec);
 }
 
+void ThumbV6MABSXOLongThunk::writeLong(uint8_t *buf) {
+  // Most Thumb instructions cannot access the high registers r8 - r15. As the
+  // only register we can corrupt is r12 we must instead spill a low register
+  // to the stack to use as a scratch register. We push r1 even though we
+  // don't need to get some space to use for the return address.
+  write16(buf + 0, 0xb403);  // push {r0, r1} ; Obtain scratch registers
+  write16(buf + 2, 0x2000);  // movs r0, :upper8_15:S
+  write16(buf + 4, 0x0200);  // lsls r0, r0, #8
+  write16(buf + 6, 0x3000);  // adds r0, :upper0_7:S
+  write16(buf + 8, 0x0200);  // lsls r0, r0, #8
+  write16(buf + 10, 0x3000); // adds r0, :lower8_15:S
+  write16(buf + 12, 0x0200); // lsls r0, r0, #8
+  write16(buf + 14, 0x3000); // adds r0, :lower0_7:S
+  write16(buf + 16, 0x9001); // str r0, [sp, #4] ; SP + 4 = S
+  write16(buf + 18, 0xbd01); // pop {r0, pc} ; restore r0 and branch to dest
+  uint64_t s = getARMThunkDestVA(destination);
+  target->relocateNoSym(buf + 2, R_ARM_THM_ALU_ABS_G3, s);
+  target->relocateNoSym(buf + 6, R_ARM_THM_ALU_ABS_G2_NC, s);
+  target->relocateNoSym(buf + 10, R_ARM_THM_ALU_ABS_G1_NC, s);
+  target->relocateNoSym(buf + 14, R_ARM_THM_ALU_ABS_G0_NC, s);
+}
+
+void ThumbV6MABSXOLongThunk::addSymbols(ThunkSection &isec) {
+  addSymbol(saver().save("__Thumbv6MABSXOLongThunk_" + destination.getName()),
+            STT_FUNC, 1, isec);
+  addSymbol("$t", STT_NOTYPE, 0, isec);
+}
+
 void ThumbV6MPILongThunk::writeLong(uint8_t *buf) {
   // Most Thumb instructions cannot access the high registers r8 - r15. As the
   // only register we can corrupt is ip (r12) we must instead spill a low
   // register to the stack to use as a scratch register.
   write16(buf + 0, 0xb401);   // P:  push {r0}        ; Obtain scratch register
   write16(buf + 2, 0x4802);   //     ldr r0, [pc, #8] ; L2
   write16(buf + 4, 0x4684);   //     mov ip, r0       ; high to low register
   write16(buf + 6, 0xbc01);   //     pop {r0}         ; restore scratch register
fatal("relocation " + toString(reloc) + " to " + toString(s) +
         " not supported for Armv5 or Armv6 targets");
 }
 
 // Create a thunk for Thumb long branch on V6-M.
 // Arm Architecture v6-M only supports Thumb instructions. This means
 // - MOVT and MOVW instructions cannot be used.
 // - Only a limited number of instructions can access registers r8 and above
 // - No interworking support is needed (all Thumb).
-static Thunk *addThunkV6M(RelType reloc, Symbol &s, int64_t a) {
+static Thunk *addThunkV6M(const InputSection &isec, RelType reloc, Symbol &s,
+                          int64_t a) {
+  const auto isPureCode = isec.getParent()->flags & SHF_ARM_PURECODE;
   switch (reloc) {
   case R_ARM_THM_JUMP19:
   case R_ARM_THM_JUMP24:
   case R_ARM_THM_CALL:
-    if (config->isPic)
+    if (config->isPic) {
+      if (!isPureCode)
         return make<ThumbV6MPILongThunk>(s, a);
+      
+      fatal("relocation " + toString(reloc) + " to " + toString(s) +
+            " not supported for Armv6-M targets for position independant"
+            " and execute only code");
+    }
+    if (isPureCode)
+      return make<ThumbV6MABSXOLongThunk>(s, a);
     return make<ThumbV6MABSLongThunk>(s, a);
   }
   fatal("relocation " + toString(reloc) + " to " + toString(s) +
         " not supported for Armv6-M targets");
 }
 
 // Creates a thunk for Thumb-ARM interworking or branch range extension.
-static Thunk *addThunkArm(RelType reloc, Symbol &s, int64_t a) {
+static Thunk *addThunkArm(const InputSection &isec, RelType reloc, Symbol &s,
+                          int64_t a) {
   // Decide which Thunk is needed based on:
   // Available instruction set
   // - An Arm Thunk can only be used if Arm state is available.
   // - A Thumb Thunk can only be used if Thumb state is available.
   // - Can only use a Thunk if it uses instructions that the Target supports.
   // Relocation is branch or branch and link
   // - Branch instructions cannot change state, can only select Thunk that
   //   starts in the same state as the caller.
   // - Branch and link relocations can change state, can select Thunks from
   //   either Arm or Thumb.
   // Position independent Thunks if we require position independent code.
+  // Execute Only Thunks if the output section is execute only code.
 
   // Handle architectures that have restrictions on the instructions that they
   // can use in Thunks. The flags below are set by reading the BuildAttributes
   // of the input objects. InputFiles.cpp contains the mapping from ARM
   // architecture to flag.
   if (!config->armHasMovtMovw) {
     if (config->armJ1J2BranchEncoding)
-      return addThunkV6M(reloc, s, a);
+      return addThunkV6M(isec, reloc, s, a);
     if (config->armHasBlx)
       return addThunkArmv5v6(reloc, s, a);
     return addThunkArmv4(reloc, s, a);
   }
 
   switch (reloc) {
   case R_ARM_PC24:
   case R_ARM_PLT32:

 Thunk *elf::addThunk(const InputSection &isec, Relocation &rel) {
   Symbol &s = *rel.sym;
   int64_t a = rel.addend;
 
   switch (config->emachine) {
   case EM_AARCH64:
     return addThunkAArch64(rel.type, s, a);
   case EM_ARM:
-    return addThunkArm(rel.type, s, a);
+    return addThunkArm(isec, rel.type, s, a);
   case EM_AVR:
     return addThunkAVR(rel.type, s, a);
   case EM_MIPS:
     return addThunkMips(rel.type, s);
   case EM_PPC:
     return addThunkPPC32(isec, rel, s);
   case EM_PPC64:
     return addThunkPPC64(rel.type, s, a);
   default:
     llvm_unreachable("add Thunk only supported for ARM, AVR, Mips and PowerPC");
   }
 }

lld/test/ELF/arm-thumb-thunk-v6m-xo.s

+// REQUIRES: arm
+// RUN: rm -rf %t && split-file %s %t
+// RUN: llvm-mc -arm-add-build-attributes -filetype=obj -triple=armv6m-none-eabi %t/a.s -o %t/a.o
+// RUN: ld.lld --no-rosegment --script %t/a.t %t/a.o -o %t/a
+// RUN: llvm-objdump --no-print-imm-hex -d %t/a --triple=armv6m-none-eabi | FileCheck %s
+// RUN: not ld.lld --no-rosegment --script %t/a.t %t/a.o -o %t/a2 --pie 2>&1 | FileCheck --check-prefix=CHECK-PI %s
+// RUN: rm -f %t/a %t/a2
+
+// Range extension thunks for Arm Architecture v6m. Only Thumb instructions
+// are permitted which limits the access to instructions that can access the
+// high registers (r8 - r15), this means that the thunks have to spill
+// low registers (r0 - r7) in order to perform the transfer of control.
+
+//--- a.t
+SECTIONS {
+  .text_low  0x11345670 : { *(.text_low) }
+  .text_high 0x12345678 : { *(.text_high) }
+}
+
+//--- a.s
+// The 'y' on the .section directive  means that this section is eXecute Only code
+ .syntax unified
+ .section .text_low, "axy", %progbits
+ .thumb
+ .type _start, %function
+ .balign 4
+ .globl _start
+_start:
+ bl far
+
+ .section .text_high, "ax", %progbits
+ .globl far
+ .type far, %function
+far:
+ bx lr
+
+// CHECK: Disassembly of section .text_low:
+// CHECK-EMPTY:
+// CHECK-NEXT: <_start>:
+// CHECK-NEXT: 11345670:        f000 f800       bl      0x11345674 <__Thumbv6MABSXOLongThunk_far>
+// CHECK: <__Thumbv6MABSXOLongThunk_far>:
+// CHECK-NEXT: 11345674:        b403    push    {r0, r1}
+// CHECK-NEXT: 11345676:        2012    movs    r0, #18
+// CHECK-NEXT: 11345678:        0200    lsls    r0, r0, #8
+// CHECK-NEXT: 1134567a:        3034    adds    r0, #52
+// CHECK-NEXT: 1134567c:        0200    lsls    r0, r0, #8
+// CHECK-NEXT: 1134567e:        3056    adds    r0, #86
+// CHECK-NEXT: 11345680:        0200    lsls    r0, r0, #8
+// CHECK-NEXT: 11345682:        3079    adds    r0, #121
+// CHECK-NEXT: 11345684:        9001    str     r0, [sp, #4]
+// CHECK-NEXT: 11345686:        bd01    pop     {r0, pc}
+// CHECK: Disassembly of section .text_high:
+// CHECK-EMPTY:
+// CHECK-NEXT: <far>:
+// CHECK-NEXT: 12345678:        4770    bx      lr
+
+// CHECK-PI:  error: relocation R_ARM_THM_CALL to far not supported for Armv6-M targets for position independant and execute only code