Implementation of R_ARM_TARGET1

[lenykholodov]

Summary

This patch provides implementation of R_ARM_TARGET1 relocation with configuration of its behaviour from a command line.

The R_ARM_TARGET1 relocation is processed in platform specific manner. Depending of the libc configuration it may be implemented as R_ARM_ABS32 or R_ARM_REL32. The decision which exact relocation should be used instead of R_ARM_TARGET1 is made immediately before the lld launch according to corresponding libc configuration. For example, linaro glibc expects R_ARM_ABS32 for R_ARM_TARGET1 relocation. It is flexible to configure lld with corresponding substitution for R_ARM_TARGET1 relocation from a command line (in the same way as ld utility works).

This patch provides two command line options for GnuLd driver: --arm-target1-rel and --arm-target1-abs (similar to ld option names with extra prefix 'arm-'). So user may choose which behaviour of R_ARM_TARGET1 is preferred for his implementation of libc (by default lld is using R_ARM_ABS32 relocation for R_ARM_TARGET1).

During the work of this patch there was an issue with implementation of command line options. They couldn't be implemented only in the driver file (GnuLdDriver.cpp) because they are target specific and should be ignored (and user should be warned about wrong using) for all targets except of armelf_linux_eabi. In this patch processing of target specific command line arguments has been divided on three parts:

1. general processing of target specific options inside the driver (GnuLdDriver.cpp): it passes all arguments from grp_targetopts group to the ELFLinkingContext::applyTargetSpecificArgument method;
2. printing warning about unknown target specific arguments in the implementation of ELFLinkingContext::applyTargetSpecificArgument;
3. actual processing of incoming target specific arguments in the ARMLinkingContext::applyTargetSpecificArgument method (inside the lib\ReaderWriter\ELF\ARM component).

Thus, if the ARMLinkingContext::applyTargetSpecificArgument is called with known target specific argument (arm-target1-rel or arm-target1-abs), it processes it byself; in case of unknown argument, this method passes it to ELFLinkingContext::applyTargetSpecificArgument which finally warns user about mistake in command line. This solution also may be used in future for implementation of target specific arguments for another targets.

[ruiu]

lib/Driver/GnuLdDriver.cpp

430 ↗

(On Diff #22897)

There's no ambiguity on command line option meaning (--arm-target1-rel and --arm-target1-abs clearly for ARM only). Also in this context we already know what the target architecture is. So I don't think we need to abstract the thing away from the driver.

You can just check value of "triple" to see if it's ARM, and if it's ARM, call setTargetRel1(true or false) directly here. If it's not ARM, print out a warning message.

lib/ReaderWriter/ELF/ARM/ARMLinkingContext.cpp

40 ↗

(On Diff #22897)

This parenthesis position style doesn't follow the LLVM coding style.

lib/ReaderWriter/ELF/ARM/ARMLinkingContext.h

56 ↗

(On Diff #22897)

Let's use C++11 initializer.

bool _target1Rel = false;

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h

26 ↗

(On Diff #22897)

Remove newline after comma.

Maybe you want to pass a boolean value instead of a context object itself? That makes it clear that you only need the boolean flag.

[lenykholodov]

lib/Driver/GnuLdDriver.cpp

430 ↗

(On Diff #22897)

Ok. I'll move all command line processing stuff to driver class. Also, I'll remove applyTargetSpecificArg polymorphism.

lib/ReaderWriter/ELF/ARM/ARMLinkingContext.cpp

40 ↗

(On Diff #22897)

Will be fixed.

lib/ReaderWriter/ELF/ARM/ARMLinkingContext.h

56 ↗

(On Diff #22897)

WIll be fixed.

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h

26 ↗

(On Diff #22897)

You are right. Will be fixed.

[ruiu]

Please upload a new patch when you are done. Thanks.

[lenykholodov]

Updates in the patch:

• all command line options processing logic has been moved to GnuLdDriver class;
• fixes in formatting and style.

[ruiu]

lib/Driver/GnuLdDriver.cpp

547 ↗

(On Diff #22987)

Instead of defining two local variables in one statement, split them into two statements. It's what we usually do in LLVM.

551 ↗

(On Diff #22987)

In this file we can safely assume that we are handling ELF, so you can remove this line.

559 ↗

(On Diff #22987)

Use hasArmTarget1Rel instead.

562 ↗

(On Diff #22987)

Use hasArmTarget1Abs instead.

[lenykholodov]

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h

26 ↗

(On Diff #22897)

Unfortunately, the ARMRelocationHandler class initializes before the command line arguments parsing. So when I'm passing bool variable to the ARMRelocationHandler construtor, this variable contains only a default value. I've rewritten the code. Now it retrieves ELFLinkingContext through _armLayout field which has been already added to the class ARMRelocationHandler before my patch and has access to the ELFLinkingContext.

[shankarke]

lib/Driver/GnuLdOptions.td

306–312 ↗

(On Diff #22987)

I dont see these options in the bfd or the gold linker. Do we need to really have these options ?

<snip>

$ ld.bfd --help | grep arm
$ ld.gold --help | grep arm

--fix-arm1176               (ARM only) Fix binaries for ARM1176 erratum.
--no-fix-arm1176            (ARM only) Do not fix binaries for ARM1176 erratum.

</snip>

[lenykholodov]

lib/Driver/GnuLdOptions.td

306–312 ↗

(On Diff #22987)

It uses them without 'arm-' prefix. I've added it to emphasize their architecture specific nature.

[lenykholodov]

lib/Driver/GnuLdOptions.td

306–312 ↗

(On Diff #22987)

You can find them in the file ld\emultempl\armelf.em:

fprintf (file, _("  --target1-rel               Interpret R_ARM_TARGET1 as R_ARM_REL32\n"));
fprintf (file, _("  --target1-abs               Interpret R_ARM_TARGET1 as R_ARM_ABS32\n"));

[lenykholodov]

Small changes in command line options processing logic according to the review comments.

[ruiu]

LGTM. Please update the commit message before committing.

lib/Driver/GnuLdDriver.cpp

563 ↗

(On Diff #22996)

nit: can you make these two "if"s into two "else if"s in order to reduce nesting level?

[denis-protivensky]

lib/Driver/GnuLdDriver.cpp

549 ↗

(On Diff #22996)

You can remove this outermost if (hasArmTarget1Rel || hasArmTarget1Abs) check completely because it doesn't introduce any logic. All the cases are double-checked inside it.

[lenykholodov]

• rebase with master;
• small updates for decreasing nesting level.

[ruiu]

Please submit.

lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h

public:
 
   const llvm::StringSet<> &wrapCalls() const { return _wrapCalls; }
 
   void setUndefinesResolver(std::unique_ptr<File> resolver);
 
   script::Sema &linkerScriptSema() { return _linkerScriptSema; }
   const script::Sema &linkerScriptSema() const { return _linkerScriptSema; }
 
+  // Set R_ARM_TARGET1 relocation behaviour
+  bool armTarget1Rel() const { return _armTarget1Rel; }
+  void setArmTarget1Rel(bool value) { _armTarget1Rel = value; }
+
 protected:
   ELFLinkingContext(llvm::Triple triple, std::unique_ptr<TargetHandler> handler)
       : _triple(triple), _targetHandler(std::move(handler)) {}
 
   Writer &writer() const override;
 
   /// Method to create a internal file for an undefined symbol
   std::unique_ptr<File> createUndefinedSymbolFile() const override;
protected:
   bool _useShlibUndefines = true;
   bool _dynamicLinkerArg = false;
   bool _noAllowDynamicLibraries = false;
   bool _mergeRODataToTextSegment = true;
   bool _demangle = true;
   bool _stripSymbols = false;
   bool _alignSegments = true;
   bool _collectStats = false;
+  bool _armTarget1Rel = false;
   uint64_t _maxPageSize = 0x1000;
 
   OutputMagic _outputMagic;
   StringRefVector _inputSearchPaths;
   std::unique_ptr<Writer> _writer;
   llvm::Optional<StringRef> _dynamicLinkerPath;
   StringRef _initFunction = "_init";
   StringRef _finiFunction = "_fini";

lld/trunk/lib/Driver/GnuLdDriver.cpp

if (auto *arg = parsedArgs->getLastArg(OPT_init))
     ctx->setInitFunction(arg->getValue());
 
   if (auto *arg = parsedArgs->getLastArg(OPT_fini))
     ctx->setFiniFunction(arg->getValue());
 
   if (auto *arg = parsedArgs->getLastArg(OPT_output_filetype))
     ctx->setOutputFileType(arg->getValue());
 
+  // Process ELF/ARM specific options
+  bool hasArmTarget1Rel = parsedArgs->hasArg(OPT_arm_target1_rel);
+  bool hasArmTarget1Abs = parsedArgs->hasArg(OPT_arm_target1_abs);
+  if (triple.getArch() == llvm::Triple::arm) {
+    if (hasArmTarget1Rel && hasArmTarget1Abs) {
+      diag << "error: options --arm-target1-rel and --arm-target1-abs"
+              " can't be used together.\n";
+      return false;
+    } else if (hasArmTarget1Rel || hasArmTarget1Abs) {
+      ctx->setArmTarget1Rel(hasArmTarget1Rel && !hasArmTarget1Abs);
+    }
+  } else if (hasArmTarget1Rel) {
+    diag << "warning: ignoring unsupported ARM/ELF specific argument: "
+         << "--arm-target1-rel\n";
+  } else if (hasArmTarget1Abs) {
+    diag << "warning: ignoring unsupported ARM/ELF specific argument: "
+         << "--arm-target1-abs\n";
+  }
+
   for (auto *arg : parsedArgs->filtered(OPT_L))
     ctx->addSearchPath(arg->getValue());
 
   // Add the default search directory specific to the target.
   if (!parsedArgs->hasArg(OPT_nostdlib))
     addPlatformSearchDirs(*ctx, triple, baseTriple);
 
   for (auto *arg : parsedArgs->filtered(OPT_u))

lld/trunk/lib/Driver/GnuLdOptions.td

def grp_extns : OptionGroup<"opts">,
      HelpText<"Extensions">;
 def output_filetype: Separate<["--"], "output-filetype">,
       HelpText<"Specify what type of output file that lld creates, YAML/Native">,
       Group<grp_extns>;
 def alias_output_filetype: Joined<["--"], "output-filetype=">,
       Alias<output_filetype>;
 
 //===----------------------------------------------------------------------===//
+/// Target Specific Options
+//===----------------------------------------------------------------------===//
+def grp_targetopts : OptionGroup<"opts">,
+     HelpText<"ARCH SPECIFIC OPTIONS">;
+def arm_target1_rel : Flag<["--"], "arm-target1-rel">,
+     Group<grp_targetopts>, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_REL32">;
+def arm_target1_abs : Flag<["--"], "arm-target1-abs">,
+     Group<grp_targetopts>, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_ABS32">;
+
+//===----------------------------------------------------------------------===//
 /// Ignored options
 //===----------------------------------------------------------------------===//
 def grp_ignored: OptionGroup<"ignored">,
      HelpText<"GNU Options ignored for Compatibility ">;
 def dashg : Flag<["-"], "g">,
      HelpText<"Ignored.">,
      Group<grp_ignored>;
 def Qy : Flag<["-"], "Qy">,

lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

static Reference::Addend readAddend_THM_JUMP11(const uint8_t *location) {
   return llvm::SignExtend32<12>(imm11 << 1);
 }
 
 static Reference::Addend readAddend(const uint8_t *location,
                                     Reference::KindValue kindValue) {
   switch (kindValue) {
   case R_ARM_ABS32:
   case R_ARM_REL32:
+  case R_ARM_TARGET1:
   case R_ARM_GOT_BREL:
   case R_ARM_BASE_PREL:
   case R_ARM_TLS_IE32:
   case R_ARM_TLS_LE32:
     return (int32_t)read32le(location);
   case R_ARM_PREL31:
     return (int32_t)(read32le(location) & 0x7FFFFFFF);
   case R_ARM_THM_CALL:
std::error_code ARMTargetRelocationHandler::applyRelocation(
   case R_ARM_ABS32:
     relocR_ARM_ABS32(location, relocVAddress, targetVAddress, addend,
                      addressesThumb);
     break;
   case R_ARM_REL32:
     relocR_ARM_REL32(location, relocVAddress, targetVAddress, addend,
                      addressesThumb);
     break;
+  case R_ARM_TARGET1:
+    if (_armLayout.target1Rel())
+      relocR_ARM_REL32(location, relocVAddress, targetVAddress, addend,
+                       addressesThumb);
+    else
+      relocR_ARM_ABS32(location, relocVAddress, targetVAddress, addend,
+                       addressesThumb);
+    break;
   case R_ARM_THM_CALL:
     // TODO: consider adding bool variable to disable J1 & J2 for archs
     // before ARMv6
     relocR_ARM_THM_CALL(location, relocVAddress, targetVAddress, addend, true,
                         addressesThumb);
     break;
   case R_ARM_CALL:
     relocR_ARM_CALL(location, relocVAddress, targetVAddress, addend,

lld/trunk/lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h

for (const auto &phdr : *this->_programHeader) {
       if (phdr->p_type == llvm::ELF::PT_TLS) {
         _tpOff = llvm::RoundUpToAlignment(TCB_SIZE, phdr->p_align);
         return *_tpOff;
       }
     }
     llvm_unreachable("TLS segment not found");
   }
 
+  bool target1Rel() const { return _ctx.armTarget1Rel(); }
+
 private:
   // TCB block size of the TLS.
   enum { TCB_SIZE = 0x8 };
 
 private:
   // Cached value of the GOT origin symbol address.
   llvm::Optional<uint64_t> _gotSymAddr;
 

lld/trunk/test/elf/ARM/rel-arm-target1.test

+# Check handling of R_ARM_TARGET1 relocation.
+#
+# 1. R_ARM_TARGET1 is equal to R_ARM_ABS32
+# RUN: yaml2obj -format=elf -docnum 1 %s > %t-abs32.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi -Bstatic \
+# RUN: --noinhibit-exec --arm-target1-abs %t-abs32.o -o %t-abs32
+# RUN: llvm-objdump -s -t %t-abs32 | FileCheck -check-prefix=CHECK-ABS32 %s
+#
+# CHECK-ABS32: Contents of section .data:
+# CHECK-ABS32-NEXT: 401000 84004000
+# CHECK-ABS32: SYMBOL TABLE:
+# CHECK-ABS32: 00400074 g     F .text  {{[0-9a-f]+}} main
+# CHECK-ABS32: 00401000 g       .data  00000004 data
+#
+# 2. R_ARM_TARGET1 is equal to R_ARM_REL32
+# Check handling of R_ARM_REL32 relocation.
+# RUN: yaml2obj -format=elf %s -docnum 2 > %t-rel32.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi -Bstatic \
+# RUN: --noinhibit-exec --arm-target1-rel %t-rel32.o -o %t-rel32
+# RUN: llvm-objdump -s -t %t-rel32 | FileCheck -check-prefix=CHECK-REL32 %s
+#
+# CHECK-REL32: Contents of section .text:
+# CHECK-REL32-NEXT: 400074 {{[0-9a-f]+}} 880fff00
+# CHECK-REL32: SYMBOL TABLE:
+# CHECK-REL32: 00400074 g     F .text {{[0-9a-f]+}} main
+# CHECK-REL32: 00401000 g       .bss  {{[0-9a-f]+}} _myref
+
+# abs32.o
+---
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         04B02DE500B08DE20030A0E30300A0E100D04BE204B09DE41EFF2FE1
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         '10000000'
+  - Name:            .rel.data
+    Type:            SHT_REL
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .data
+    Relocations:
+      - Offset:          0x0000000000000000
+        Symbol:          main
+        Type:            R_ARM_TARGET1
+        Addend:          0
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+Symbols:
+  Global:
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+      Size:            0x000000000000001C
+    - Name:            data
+      Type:            STT_OBJECT
+      Section:         .data
+      Size:            0x0000000000000004
+# rel32.o
+---
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         80B400AF0000FF0000231846BD465DF8047B7047
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .text
+    Relocations:
+      - Offset:          0x0000000000000004
+        Symbol:          _myref
+        Type:            R_ARM_TARGET1
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         ''
+Symbols:
+  Global:
+    - Name:            _myref
+      Type:            STT_OBJECT
+      Section:         .bss
+      Size:            0x0000000000000004
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000001
+      Size:            0x0000000000000014
+...

lld/trunk/test/elf/options/target-specific-args.test

+# Test of target specific options
+# RUN: lld -flavor gnu -target x86_64 --arm-target1-rel \
+# RUN: %p/../Inputs/foo.o.x86-64 --noinhibit-exec -o %t 2>&1 | FileCheck %s
+
+# CHECK: warning: ignoring unsupported ARM/ELF specific argument: --arm-target1-rel