[PATCH] D54854: [LLD][AArch64] Cortex-a53-843419 erratum should not apply to relaxed TLS.

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 23 03:56:56 PST 2018


peter.smith created this revision.
peter.smith added reviewers: ruiu, grimar.
Herald added subscribers: kristof.beyls, arichardson, javed.absar, emaste.
Herald added a reviewer: espindola.

The changes to the instructions performed by TLS relaxation and the errata patching are performed with relocations. As these are applied so late the errata scanning won't see the changes in the section data made by the TLS relaxation. This can lead to a TLS relaxed sequence being patched when it doesn't need to be.

      

The fix checks to see if there is a R_RELAX_TLS_IE_TO_LE instruction at the same address as the ADRP as this indicates the presence of a relaxation of a sequence that might get recognised as a patch. None of the other TLS code sequences that can be relaxed by LLD will match the erratum so I've not tested for them.

This fix was inspired by a recently reported GNU ld assertion failure https://sourceware.org/bugzilla/show_bug.cgi?id=23904 . I don't think that LLD's errata scanner will match any of the TLS code sequences in that PR as LLD checks that the 2nd instruction doesn't write to the destination register of the ADRP and ld.bfd does not. However as the fix is simple I've made the change to be safe.


https://reviews.llvm.org/D54854

Files:
  ELF/AArch64ErrataFix.cpp
  test/ELF/aarch64-cortex-a53-843419-tlsrelax.s


Index: test/ELF/aarch64-cortex-a53-843419-tlsrelax.s
===================================================================
--- /dev/null
+++ test/ELF/aarch64-cortex-a53-843419-tlsrelax.s
@@ -0,0 +1,38 @@
+// REQUIRES: aarch64
+// RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %s -o %t.o
+// RUN: ld.lld -fix-cortex-a53-843419 %t.o -o %t2
+// RUN: llvm-objdump -triple=aarch64-linux-gnu -d %t2 | FileCheck %s
+
+// The following code sequence is covered by the TLS IE to LE relaxation. It
+// transforms the ADRP, LDR to MOVZ, MOVK. The former can trigger a
+// cortex-a53-843419 patch, whereas the latter can not. As both
+// relaxation and patching transform instructions very late in the
+// link there is a possibility of them both being simultaneously
+// applied. In this case the relaxed sequence is immune from the erratum so we
+// prefer to keep it.
+ .text
+ .balign 4096
+ .space  4096 - 8
+ .globl _start
+ .type  _start, at function
+_start:
+ mrs    x1, tpidr_el0
+ adrp   x0, :gottprel:v
+ ldr    x1, [x0, #:gottprel_lo12:v]
+ adrp   x0, :gottprel:v
+ ldr    x1, [x0, #:gottprel_lo12:v]
+ ret
+
+// CHECK: _start:
+// CHECK-NEXT:   210ff8:        41 d0 3b d5     mrs     x1, TPIDR_EL0
+// CHECK-NEXT:   210ffc:        00 00 a0 d2     movz    x0, #0, lsl #16
+// CHECK-NEXT:   211000:        01 02 80 f2     movk    x1, #16
+// CHECK-NEXT:   211004:        00 00 a0 d2     movz    x0, #0, lsl #16
+// CHECK-NEXT:   211008:        01 02 80 f2     movk    x1, #16
+// CHECK-NEXT:   21100c:        c0 03 5f d6     ret
+
+ .type  v, at object
+ .section       .tbss,"awT", at nobits
+ .globl v
+v:
+ .word 0
Index: ELF/AArch64ErrataFix.cpp
===================================================================
--- ELF/AArch64ErrataFix.cpp
+++ ELF/AArch64ErrataFix.cpp
@@ -538,20 +538,24 @@
                            InputSection *IS,
                            std::vector<Patch843419Section *> &Patches) {
   // There may be a relocation at the same offset that we are patching. There
-  // are three cases that we need to consider.
+  // are four cases that we need to consider.
   // Case 1: R_AARCH64_JUMP26 branch relocation. We have already patched this
   // instance of the erratum on a previous patch and altered the relocation. We
   // have nothing more to do.
-  // Case 2: A load/store register (unsigned immediate) class relocation. There
+  // Case 2: A TLS Relaxation R_RELAX_TLS_IE_TO_LE. In this case the ADRP that
+  // we read will be transformed into a MOVZ later so we actually don't match
+  // the sequence and have nothing more to do.
+  // Case 3: A load/store register (unsigned immediate) class relocation. There
   // are two of these R_AARCH_LD64_ABS_LO12_NC and R_AARCH_LD64_GOT_LO12_NC and
   // they are both absolute. We need to add the same relocation to the patch,
   // and replace the relocation with a R_AARCH_JUMP26 branch relocation.
-  // Case 3: No relocation. We must create a new R_AARCH64_JUMP26 branch
+  // Case 4: No relocation. We must create a new R_AARCH64_JUMP26 branch
   // relocation at the offset.
   auto RelIt = std::find_if(
       IS->Relocations.begin(), IS->Relocations.end(),
       [=](const Relocation &R) { return R.Offset == PatcheeOffset; });
-  if (RelIt != IS->Relocations.end() && RelIt->Type == R_AARCH64_JUMP26)
+  if (RelIt != IS->Relocations.end() &&
+      (RelIt->Type == R_AARCH64_JUMP26 || RelIt->Expr == R_RELAX_TLS_IE_TO_LE))
     return;
 
   log("detected cortex-a53-843419 erratum sequence starting at " +


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54854.175109.patch
Type: text/x-patch
Size: 3499 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181123/2137b809/attachment.bin>


More information about the llvm-commits mailing list