[LLD][ELF] - Fix the different behavior of the linker script symbols on different platforms.

George Rimar · George Rimar · commit da49faf15e88 · 2019-04-18T08:15:54.000Z
This generalizes code and also fixes the broken behavior shown in one of our test cases for some targets, like x86-64. The issue occurs when the forward declarations are used in the script. One of the samples is: SECTIONS { foo = ADDR(.text) - ABSOLUTE(ADDR(.text)); }; In that case, we have a broken output when output target does not use thunks. That happens because thunks creating code (called from maybeAddThunks) calls Script->assignAddresses() at least one more time, what fixups the values. As a result final symbols values can be different on AArch64 and x86, for example. In this patch, I generalize and rename maybeAddThunks to finalizeAddressDependentContent and now it is used and called by all targets. Differential revision: https://reviews.llvm.org/D55550 llvm-svn: 358646
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
@@ -55,7 +55,7 @@ template <class ELFT> class Writer {
   void forEachRelSec(llvm::function_ref<void(InputSectionBase &)> Fn);
   void sortSections();
   void resolveShfLinkOrder();
-  void maybeAddThunks();
+  void finalizeAddressDependentContent();
   void sortInputSections();
   void finalizeSections();
   void checkExecuteOnly();
@@ -1460,19 +1460,15 @@ template <class ELFT> void Writer<ELFT>::resolveShfLinkOrder() {
   }
 }
 
-// For most RISC ISAs, we need to generate content that depends on the address
-// of InputSections. For example some architectures such as AArch64 use small
-// displacements for jump instructions that is the linker's responsibility for
-// creating range extension thunks for. As the generation of the content may
-// also alter InputSection addresses we must converge to a fixed point.
-template <class ELFT> void Writer<ELFT>::maybeAddThunks() {
-  if (!Target->NeedsThunks && !Config->AndroidPackDynRelocs &&
-      !Config->RelrPackDynRelocs)
-    return;
-
+// We need to generate and finalize the content that depends on the address of
+// InputSections. As the generation of the content may also alter InputSection
+// addresses we must converge to a fixed point. We do that here. See the comment
+// in Writer<ELFT>::finalizeSections().
+template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
   ThunkCreator TC;
   AArch64Err843419Patcher A64P;
 
+  // For some targets, like x86, this loop iterates only once.
   for (;;) {
     bool Changed = false;
 
@@ -1756,19 +1752,30 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // known but before addresses are allocated.
   resolveShfLinkOrder();
 
-  // Jump instructions in many ISAs have small displacements, and therefore they
-  // cannot jump to arbitrary addresses in memory. For example, RISC-V JAL
-  // instruction can target only +-1 MiB from PC. It is a linker's
-  // responsibility to create and insert small pieces of code between sections
-  // to extend the ranges if jump targets are out of range. Such code pieces are
-  // called "thunks".
+  // This is used to:
+  // 1) Create "thunks":
+  //    Jump instructions in many ISAs have small displacements, and therefore
+  //    they cannot jump to arbitrary addresses in memory. For example, RISC-V
+  //    JAL instruction can target only +-1 MiB from PC. It is a linker's
+  //    responsibility to create and insert small pieces of code between
+  //    sections to extend the ranges if jump targets are out of range. Such
+  //    code pieces are called "thunks".
+  //
+  //    We add thunks at this stage. We couldn't do this before this point
+  //    because this is the earliest point where we know sizes of sections and
+  //    their layouts (that are needed to determine if jump targets are in
+  //    range).
+  //
+  // 2) Update the sections. We need to generate content that depends on the
+  //    address of InputSections. For example, MIPS GOT section content or
+  //    android packed relocations sections content.
   //
-  // We add thunks at this stage. We couldn't do this before this point because
-  // this is the earliest point where we know sizes of sections and their
-  // layouts (that are needed to determine if jump targets are in range).
-  maybeAddThunks();
+  // 3) Assign the final values for the linker script symbols. Linker scripts
+  //    sometimes using forward symbol declarations. We want to set the correct
+  //    values. They also might change after adding the thunks.
+  finalizeAddressDependentContent();
 
-  // maybeAddThunks may have added local symbols to the static symbol table.
+  // finalizeAddressDependentContent may have added local symbols to the static symbol table.
   finalizeSynthetic(In.SymTab);
   finalizeSynthetic(In.PPC64LongBranchTarget);
 
diff --git a/lld/test/ELF/linkerscript/addr-zero.test b/lld/test/ELF/linkerscript/addr-zero.test
@@ -1,21 +1,27 @@
-# REQUIRES: x86
+# REQUIRES: x86, aarch64
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux /dev/null -o %t.o
 # RUN: ld.lld -o %t.so --script %s %t.o -shared
 # RUN: llvm-readobj --symbols %t.so | FileCheck %s
 
-# Test that the script creates a non absolute symbol with value
-# 0 I.E., a symbol that refers to the load address.
+## Test that the script creates a non absolute symbol with value
+## 0 I.E., a symbol that refers to the load address.
 
 # CHECK:      Symbol {
 # CHECK:        Name: foo
-# CHECK-NEXT:   Value: 0x70
+# CHECK-NEXT:   Value: 0x0
 # CHECK-NEXT:   Size: 0
 # CHECK-NEXT:   Binding: Global
 # CHECK-NEXT:   Type: None
 # CHECK-NEXT:   Other: 0
 # CHECK-NEXT:   Section: .text
 # CHECK-NEXT: }
 
+## Because of a bug we had a different behavior (different symbol 'foo' value)
+## on a platforms that might use thunks, like AArch64. Check that issue is fixed.
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnux /dev/null -o %t.o
+# RUN: ld.lld -o %t.so --script %s %t.o -shared
+# RUN: llvm-readobj --symbols %t.so | FileCheck %s
+
 SECTIONS {
   foo = ADDR(.text) - ABSOLUTE(ADDR(.text));
 };
