[PATCH] D42054: [ELF] - Stop mixing order of -defsym/-script commands.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: espindola, ruiu.
Herald added a subscriber: emaste.
grimar updated this revision to Diff 129820.
grimar added a comment.

- Fixed comment.


Previously we always handled -defsym after other commands in command line.
That made impossible to overload values set by -defsym from linker script:

   test.script:            
    foo = 0x22;
  -defsym=foo=0x11 -script t.script

would always set foo to 0x11.

That is inconstent with common logic which allows to override command line
options. it is inconsistent with bfd behavior and seems breaks assumption that
-defsym is the same as linker script assignment, as -defsyms always handled out of
command line order.

Patch fixes the handling order.


https://reviews.llvm.org/D42054

Files:
  ELF/Driver.cpp
  test/ELF/linkerscript/defsym.s


Index: test/ELF/linkerscript/defsym.s
===================================================================
--- test/ELF/linkerscript/defsym.s
+++ test/ELF/linkerscript/defsym.s
@@ -0,0 +1,19 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "foo = 0x22;" > %t.script
+
+## This testcase checks that we apply -defsym and linker script
+## in the same order are they specified in a command line. 
+
+## Check that linker script can override -defsym assignments.
+# RUN: ld.lld %t.o -defsym=foo=0x11 -script %t.script -o %t
+# RUN: llvm-readobj -t %t | FileCheck %s
+# CHECK:      Name: foo
+# CHECK-NEXT:   Value: 0x22
+
+## Check that -defsym can override linker script. Check that multiple
+## -defsym commands for the same symbol are allowed.
+# RUN: ld.lld %t.o -script %t.script -defsym=foo=0x11 -defsym=foo=0x33 -o %t
+# RUN: llvm-readobj -t %t | FileCheck %s --check-prefix=REORDER
+# REORDER:      Name: foo
+# REORDER-NEXT:   Value: 0x33
Index: ELF/Driver.cpp
===================================================================
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -830,6 +830,13 @@
     case OPT_INPUT:
       addFile(Arg->getValue(), /*WithLOption=*/false);
       break;
+    case OPT_defsym: {
+      StringRef From;
+      StringRef To;
+      std::tie(From, To) = StringRef(Arg->getValue()).split('=');
+      readDefsym(From, MemoryBufferRef(To, "-defsym"));
+      break;
+    }
     case OPT_script:
       if (Optional<std::string> Path = searchLinkerScript(Arg->getValue())) {
         if (Optional<MemoryBufferRef> MB = readFile(*Path))
@@ -1011,14 +1018,6 @@
   for (InputFile *F : Files)
     Symtab->addFile<ELFT>(F);
 
-  // Process -defsym option.
-  for (auto *Arg : Args.filtered(OPT_defsym)) {
-    StringRef From;
-    StringRef To;
-    std::tie(From, To) = StringRef(Arg->getValue()).split('=');
-    readDefsym(From, MemoryBufferRef(To, "-defsym"));
-  }
-
   // Now that we have every file, we can decide if we will need a
   // dynamic symbol table.
   // We need one if we were asked to export dynamic symbols or if we are


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42054.129820.patch
Type: text/x-patch
Size: 2113 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180115/334e517f/attachment.bin>