LinkerScript: Add -T <scriptfile> option

[meadori]

Summary

GNU LD has an option named -T/--script which allows a user to specify
a linker script to be used [1]. LLD already accepts linker scripts
without this option, but the option is widely used. Therefore it is
best to support it in LLD as well.

[1] https://sourceware.org/binutils/docs-2.25/ld/Options.html#Options

[davide]

lib/Driver/GnuLdOptions.td

262

is 'defm' a valid keyword in python?
Why are you not following the convention for the other options?

[davide]

lib/Driver/GnuLdOptions.td

262

OK, you can use defm here, apparently.

[rafaelauler]

This is the tablegen language, not Python :)

http://llvm.org/docs/TableGen/

I'm curious about why is "-T" being handled here as an alias for "-l". "-T" is exclusive for linker scripts and cannot accept any other input files.

Actually, currently, lld does handle -T, but in a weird way. "-T" is not recognized, thus it generates a warning, but then the parameter to "-T" is handled as a regular input and eventually gets parsed as a regular linker script.

Currently there is no difference between a script that overrides the "default linker script" and one that does not, but we need this for the future (well, mostly producing an error if the linker script contains special commands that are reserved for either the default linker script or the script that replaces it).

[meadori]

In D8138#136065, @rafaelauler wrote:

I'm curious about why is "-T" being handled here as an alias for "-l". "-T" is exclusive for linker scripts and cannot
accept any other input files.

That is an oversight on my part. I will fix it on a later revision. After taking a more detailed look at the expected
behavior of -l and -T I see where we have a related bug with -l. The following will find 'test.ls':

lld -flavor gnu -target x86_64  -ltest.ls

This is due to how we implement findFile in GnuLdDriver.cpp.
However, the colon form should work (assuming the script is in the find path
it will append to the default script):

lld -flavor gnu -target x86_64  -l:test.ls

Actually, currently, lld does handle -T, but in a weird way. "-T" is not recognized, thus it generates a warning,
but then the parameter to "-T" is handled as a regular input and eventually gets parsed as a regular linker script.

I suspect that is accidental. *Any* unrecognized options issue that warning. Not sure if that is a good idea.

Currently there is no difference between a script that overrides the "default linker script" and one that does not,
but we need this for the future (well, mostly producing an error if the linker script contains special commands that
are reserved for either the default linker script or the script that replaces it).

Exactly. Right now there isn't much of difference, but to provide a proper distinction between default and user provided
it will matter.

[meadori]

Update to fix the -lfilename bug mentioned earlier in the review and add more tests. Note that input, -l, and -T still share the same code path in the option switch, *but* the correct checking is in place (and was before) to determine when we have a linker script in the case of input and -l.

[meadori]

Any other comments on this one?

[rafaelauler]

No more comments, Meador, LGTM. Thanks for working on this.

lib/Driver/GnuLdDriver.cpp

case OPT_end_group: {
       int startGroupPos = groupStack.top();
       ctx->getNodes().push_back(
           llvm::make_unique<GroupEnd>(numfiles - startGroupPos));
       groupStack.pop();
       break;
     }
 
     case OPT_INPUT:
-    case OPT_l: {
+    case OPT_l:
+    case OPT_T: {
       bool dashL = (arg->getOption().getID() == OPT_l);
       StringRef path = arg->getValue();
 
       ErrorOr<StringRef> pathOrErr = findFile(*ctx, path, dashL);
       if (std::error_code ec = pathOrErr.getError()) {
         auto file = llvm::make_unique<ErrorFile>(path, ec);
         auto node = llvm::make_unique<FileNode>(std::move(file));
         node->setAsNeeded(asNeeded);

lib/Driver/GnuLdOptions.td

def alias_strip_all : Flag<["-"], "s">,
      Alias<strip_all>;
 defm wrap : smDash<"wrap", "wrap",
             "Use a wrapper function for symbol. Any "
             " undefined reference to symbol will be resolved to "
             "\"__wrap_symbol\". Any undefined reference to \"__real_symbol\""
             " will be resolved to symbol.">,
             MetaVarName<"<symbol>">,
             Group<grp_symbolopts>;
+
+//===----------------------------------------------------------------------===//
+/// Script Options
+//===----------------------------------------------------------------------===//
+def grp_scriptopts : OptionGroup<"opts">,
+     HelpText<"SCRIPT OPTIONS">;
+defm T : smDash<"T", "script",
+      "Use the given linker script in place of the default script.">,
+      Group<grp_scriptopts>;
+
 //===----------------------------------------------------------------------===//
 /// Optimization Options
 //===----------------------------------------------------------------------===//
 def grp_opts : OptionGroup<"opts">,
      HelpText<"OPTIMIZATIONS">;
 def hash_style : Joined <["--"], "hash-style=">,
      HelpText<"Set the type of linker's hash table(s)">,
      Group<grp_opts>;

lib/Driver/TODO.rst

 * -O
 * -q,--emit-relocs
 * --force-dynamic
 * --relocatable
 * -R,--just-symbols
 * -s,--strip-all
 * -S,--strip-debug
 * --trace
-* -T,--script
 * -dT,--default-script
 * -Ur
 * --unique
 * -v,--version,-V
 * -x,--discard-all
 * -X,--discard-locals
 * -y,--trace-symbol
 * -z (keywords need to be implemented)

lib/ReaderWriter/ELF/ELFLinkingContext.cpp

for (StringRef dir : _inputSearchPaths) {
     // Search for static libraries too
     buildSearchPath(path, dir, _sysrootPath);
     llvm::sys::path::append(path, hasColonPrefix
                                       ? libName.drop_front()
                                       : Twine("lib", libName) + ".a");
     if (llvm::sys::fs::exists(path.str()))
       return StringRef(*new (_allocator) std::string(path.str()));
   }
-  if (!llvm::sys::fs::exists(libName))
-    return make_error_code(llvm::errc::no_such_file_or_directory);
+  if (hasColonPrefix && llvm::sys::fs::exists(libName.drop_front()))
+      return libName.drop_front();
 
-  return libName;
+  return make_error_code(llvm::errc::no_such_file_or_directory);
 }
 
 ErrorOr<StringRef> ELFLinkingContext::searchFile(StringRef fileName,
                                                  bool isSysRooted) const {
   SmallString<128> path;
   if (llvm::sys::path::is_absolute(fileName) && isSysRooted) {
     path.assign(_sysrootPath);
     path.append(fileName);

test/elf/linkerscript/Inputs/valid.ls

+/* A simple valid linker script used for testing the -T/--script options.
+ *
+ * An unresolved symbol named '_entry_point' can be scanned for by the tests
+ * to determine that the linker script was processed.
+ */
+ENTRY(_entry_point)

test/elf/linkerscript/invalid-script-cli-1.test

+# Check that the -T/--script options issue an error when passed
+# filenames for files that do not exist.
+
+RUN: not lld -flavor gnu -target x86_64 -T idonotexist.ls 2> %t.err
+RUN: FileCheck %s < %t.err
+
+RUN: not lld -flavor gnu -target x86_64 --script=idonotexist.ls 2> %t.err
+RUN: FileCheck %s < %t.err
+
+CHECK: {{.*}}lld: cannot find file {{.*}}idonotexist.ls

test/elf/linkerscript/invalid-script-cli-2.test

+# Check that linker script are *not* picked up with -lscript.ls.
+
+RUN: not lld -flavor gnu -target x86_64 -L%p/Inputs/ -lvalid.ls 2> %t.err
+RUN: FileCheck %s < %t.err
+
+CHECK: {{.*}}: Unable to find library -lvalid.ls

test/elf/linkerscript/valid-script-cli.objtxt

+# Check that the linker script inputs are accepted properly.
+
+# RUN: lld -flavor gnu -target x86_64 %p/Inputs/valid.ls -r %s \
+# RUN:     --output-filetype=yaml | FileCheck %s
+
+# RUN: lld -flavor gnu -target x86_64 -T %p/Inputs/valid.ls -r %s \
+# RUN:     --output-filetype=yaml | FileCheck %s
+
+# RUN: lld -flavor gnu -target x86_64 --script=%p/Inputs/valid.ls -r %s \
+# RUN:     --output-filetype=yaml | FileCheck %s
+
+# RUN: lld -flavor gnu -target x86_64 -L%p/Inputs/ -l:valid.ls -r %s \
+# RUN:     --output-filetype=yaml | FileCheck %s
+
+defined-atoms:
+  - name:            main
+    scope:           global
+    content:         [ B8, 00, 00, 00, 00, C7, 44, 24, FC, 00, 00, 00, 00, C3 ]
+    alignment:       2^4
+    section-choice:  custom-required
+    section-name:    .text
+
+# CHECK: _entry_point