[LLD][ELF][ARM] Don't apply --fix-cortex-a8 to relocatable links.

[peter.smith]

Summary

The --fix-cortex-a8 is sensitive to alignment and the precise destination of branch instructions. These are not knowable at relocatable link time. We follow GNU ld and the --fix-cortex-a53-843419 (D72968) by not patching the code when there is a relocatable link.

[grimar]

It looks fine to me, I'll leave it for others, have a minor question though.

lld/test/ELF/arm-fix-cortex-a8-recognize.s

14

Does it make sense to check what we do with -verbose -r?

[peter.smith]

In D73100#1831139, @grimar wrote:

It looks fine to me, I'll leave it for others, have a minor question though.

lld/test/ELF/arm-fix-cortex-a8-recognize.s

14

There is a small benefit. I'll add a check that we don't see the message as it makes it more consistent with the others.

[peter.smith]

Added a check for lld -v to make sure a detection message isn't printed.

[MaskRay]

I was waiting for other opinions on D72968 because I was thinking whether it makes sense to reject -r --fix-*. It seems GNU ld has made a decision and the way is simple, let's do this.

lld/test/ELF/arm-fix-cortex-a8-recognize.s

14

-v --> --verbose

They are different options. -v just prints the version number.

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->executeOnly =
       args.hasFlag(OPT_execute_only, OPT_no_execute_only, false);
   config->exportDynamic =
       args.hasFlag(OPT_export_dynamic, OPT_no_export_dynamic, false);
   config->filterList = args::getStrings(args, OPT_filter);
   config->fini = args.getLastArgValue(OPT_fini, "_fini");
   config->fixCortexA53Errata843419 = args.hasArg(OPT_fix_cortex_a53_843419) &&
                                      !args.hasArg(OPT_relocatable);
-  config->fixCortexA8 = args.hasArg(OPT_fix_cortex_a8);
+  config->fixCortexA8 =
+      args.hasArg(OPT_fix_cortex_a8) && !args.hasArg(OPT_relocatable);
   config->forceBTI = hasZOption(args, "force-bti");
   config->gcSections = args.hasFlag(OPT_gc_sections, OPT_no_gc_sections, false);
   config->gnuUnique = args.hasFlag(OPT_gnu_unique, OPT_no_gnu_unique, true);
   config->gdbIndex = args.hasFlag(OPT_gdb_index, OPT_no_gdb_index, false);
   config->icf = getICF(args);
   config->ignoreDataAddressEquality =
       args.hasArg(OPT_ignore_data_address_equality);
   config->ignoreFunctionAddressEquality =

lld/test/ELF/arm-fix-cortex-a8-recognize.s

 // REQUIRES: arm
 // RUN: llvm-mc -filetype=obj -triple=armv7a-linux-gnueabihf --arm-add-build-attributes %s -o %t.o
 // RUN: ld.lld --fix-cortex-a8 -verbose %t.o -o %t2 2>&1 | FileCheck %s
 // RUN: llvm-objdump -d %t2 --start-address=0x1a004 --stop-address=0x1a024 --no-show-raw-insn | FileCheck --check-prefix=CHECK-PATCHES %s
 // RUN: llvm-objdump -d %t2 --start-address=0x12ffa --stop-address=0x13002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE1 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x13ffa --stop-address=0x14002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE2 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x14ffa --stop-address=0x15002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE3 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x15ff4 --stop-address=0x16002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE4 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x16ffa --stop-address=0x17002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE5 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x17ffa --stop-address=0x18002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE6 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x18ffa --stop-address=0x19002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE7 %s
 // RUN: llvm-objdump -d %t2 --start-address=0x19ff4 --stop-address=0x1a002 --no-show-raw-insn | FileCheck --check-prefix=CALLSITE8 %s
+// RUN: ld.lld --fix-cortex-a8 -verbose -r %t.o -o %t3 2>&1 | FileCheck --check-prefix=CHECK-RELOCATABLE-LLD %s
+// RUN: llvm-objdump --no-show-raw-insn -d %t3 --start-address=0xffa --stop-address=0x1002 | FileCheck --check-prefix=CHECK-RELOCATABLE %s
 
 // CHECK:      ld.lld: detected cortex-a8-657419 erratum sequence starting at 12FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 13FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 14FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 15FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 16FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 17FFE in unpatched output.
 // CHECK-NEXT: ld.lld: detected cortex-a8-657419 erratum sequence starting at 18FFE in unpatched output.
 
+/// We do not detect errors when doing a relocatable link as we don't know what
+/// the final addresses are.
+// CHECK-RELOCATABLE-LLD-NOT: ld.lld: detected cortex-a8-657419 erratum sequence
+
 /// Basic tests for the -fix-cortex-a8 erratum fix. The full details of the
 /// erratum and the patch are in ARMA8ErrataFix.cpp . The test creates an
 /// instance of the erratum every 4KiB (32-bit non-branch, followed by 32-bit
 /// branch instruction, where the branch instruction spans two 4 KiB regions,
 /// and the branch destination is in the first 4KiB region.
 ///
 /// Test each 32-bit branch b.w, bcc.w, bl, blx. For b.w, bcc.w, and bl we
 /// check the relocated and non-relocated forms. The blx instruction

 /// 32-bit Branch spans 2 4KiB regions, preceded by a 32-bit non branch
 /// instruction, expect a patch.
  nop.w
  b.w target
 
 // CALLSITE1:      00012ffa target:
 // CALLSITE1-NEXT:    12ffa:            nop.w
 // CALLSITE1-NEXT:    12ffe:            b.w     #28674
+/// Expect no patch when doing a relocatable link ld -r.
+// CHECK-RELOCATABLE: 00000ffa target:
+// CHECK-RELOCATABLE-NEXT:      ffa:            nop.w
+// CHECK-RELOCATABLE-NEXT:      ffe:            b.w     #-4
 
  .space 4088
  .type target2, %function
  .local target2
 target2:
 /// 32-bit Branch and link spans 2 4KiB regions, preceded by a 32-bit
 /// non branch instruction, expect a patch.
  nop.w