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

[Rafael Avila de Espindola via llvm-commits]

LGTM.

It is a nice simplification that we handle -defsym as a mini linker
script, so I think this makes lld more self consistent.

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 129820.
> grimar added a comment.
>
> - Fixed comment.
>
>
> 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
>
>
> 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