[ELF] Fall back to search dirs for linker scripts specified with -T

[arichardson]

Summary

This matches the behaviour of ld.bfd:
https://sourceware.org/binutils/docs/ld/Options.html#Options

If scriptfile does not exist in the current directory, ld looks for it in
the directories specified by any preceding ‘-L’ options. Multiple ‘-T’
options accumulate.

[ruiu]

ELF/Driver.cpp

867 ↗

(On Diff #123180)

Early-break after this line.

ELF/DriverUtils.cpp

223 ↗

(On Diff #123180)

By convention, we don't have a blank line between a function comment and a function definition.

ELF/ScriptParser.cpp

355 ↗

(On Diff #123180)

Please return after this line. In general, we prefer early return/break/continue.

[grimar]

Thanks for this patch, few comments/suggestions below.

ELF/DriverUtils.cpp

220 ↗

(On Diff #123180)

I think should be script file as it is not a single word.
btw, can we just copy paste the GNU spec ?

I think we usually just describe LLD behavior and mention it agrees/disagrees with gnu linkers.
I do not think putting URLs here is very useful in that particular case as behavior is simple.
So I would rewrite this so that will something that just describes the behavior.

test/ELF/linkerscript/linker-script-in-search-path.s

1 ↗

(On Diff #123180)

Could you put this below REQUIRES ? I think that is more consistent with other tests,
where REQURES is usually a first line. Probably that is not always true, but anyways..

13 ↗

(On Diff #123180)

Please put ERROR check line right after this one, that way it allows keep check calls and check lines
be close to each other and look more isolated and convinent to read:

# If the linker script specified with -T is missing we should emit an error
# RUN: not ld.lld -Tfoo.script %t.o 2>&1 | FileCheck %s -check-prefix ERROR
# ERROR: error: cannot find linker script foo.script

The same for CHECK

23 ↗

(On Diff #123180)

Does not seem you need any code here.

26 ↗

(On Diff #123180)

You need new line here, that is consistent with other testcases.

[arichardson]

addressed reviews

[grimar]

I have no futher comments, thanks !

[grimar]

(please wait approvement from Rui)

[arichardson]

ELF/DriverUtils.cpp

220 ↗

(On Diff #123180)

I just copied those two sentences from the bfd manual so I didn't bother to fix any typos. Anyway, I've deleted it now.

[ruiu]

LGTM with these changes.

ELF/DriverUtils.cpp

217 ↗

(On Diff #123329)

Remove else since the last if ends with return.

219 ↗

(On Diff #123329)

Instead of returning None, directly return findFromSearchPaths(Name).

test/ELF/linkerscript/linker-script-in-search-path.s

20 ↗

(On Diff #123329)

Please remove the redundant trailing newline.

lld/trunk/ELF/Driver.h

 #include "Options.inc"
 #undef OPTION
 };
 
 void printHelp(const char *Argv0);
 std::string createResponseFile(const llvm::opt::InputArgList &Args);
 
 llvm::Optional<std::string> findFromSearchPaths(StringRef Path);
+llvm::Optional<std::string> searchLinkerScript(StringRef Path);
 llvm::Optional<std::string> searchLibrary(StringRef Path);
 
 } // namespace elf
 } // namespace lld
 
 #endif

lld/trunk/ELF/Driver.cpp

for (auto *Arg : Args) {
     switch (Arg->getOption().getUnaliasedOption().getID()) {
     case OPT_library:
       addLibrary(Arg->getValue());
       break;
     case OPT_INPUT:
       addFile(Arg->getValue(), /*WithLOption=*/false);
       break;
     case OPT_script:
-      if (Optional<MemoryBufferRef> MB = readFile(Arg->getValue()))
+      if (Optional<std::string> Path = searchLinkerScript(Arg->getValue())) {
+        if (Optional<MemoryBufferRef> MB = readFile(*Path))
           readLinkerScript(*MB);
         break;
+      }
+      error(Twine("cannot find linker script ") + Arg->getValue());
+      break;
     case OPT_as_needed:
       Config->AsNeeded = true;
       break;
     case OPT_format:
       InBinary = getBinaryOption(Arg->getValue());
       break;
     case OPT_no_as_needed:
       Config->AsNeeded = false;

lld/trunk/ELF/DriverUtils.cpp

for (StringRef Dir : Config->SearchPaths) {
     if (!Config->Static)
       if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".so"))
         return S;
     if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".a"))
       return S;
   }
   return None;
 }
+
+// If a linker script doesn't exist in the current directory, we also look for
+// the script in the '-L' search paths. This matches the behaviour of both '-T'
+// and linker script INPUT() directives in ld.bfd.
+Optional<std::string> elf::searchLinkerScript(StringRef Name) {
+  if (fs::exists(Name))
+    return Name.str();
+  return findFromSearchPaths(Name);
+}

lld/trunk/ELF/ScriptParser.cpp

 void ScriptParser::readInclude() {
   StringRef Tok = unquote(next());
 
   if (!Seen.insert(Tok).second) {
     setError("there is a cycle in linker script INCLUDEs");
     return;
   }
 
-  // https://sourceware.org/binutils/docs/ld/File-Commands.html:
-  // The file will be searched for in the current directory, and in any
-  // directory specified with the -L option.
-  if (sys::fs::exists(Tok)) {
-    if (Optional<MemoryBufferRef> MB = readFile(Tok))
-      tokenize(*MB);
-    return;
-  }
-  if (Optional<std::string> Path = findFromSearchPaths(Tok)) {
+  if (Optional<std::string> Path = searchLinkerScript(Tok)) {
     if (Optional<MemoryBufferRef> MB = readFile(*Path))
       tokenize(*MB);
     return;
   }
-  setError("cannot open " + Tok);
+  setError("cannot find linker script " + Tok);
 }
 
 void ScriptParser::readOutput() {
   // -o <file> takes predecence over OUTPUT(<file>).
   expect("(");
   StringRef Tok = next();
   if (Config->OutputFile.empty())
     Config->OutputFile = unquote(Tok);

lld/trunk/test/ELF/invalid-linkerscript.test

 
 # RUN: echo "EXTERN xyz" > %t6
 # RUN: not ld.lld %t6 no-such-file 2>&1 | FileCheck -check-prefix=ERR6 %s
 # ERR6: ( expected, but got xyz
 # ERR6: cannot open no-such-file:
 
 # RUN: echo "INCLUDE /no/such/file" > %t7
 # RUN: not ld.lld %t7 no-such-file 2>&1 | FileCheck -check-prefix=ERR7 %s
-# ERR7: cannot open /no/such/file
+# ERR7: cannot find linker script /no/such/file
 # ERR7: cannot open no-such-file:
 
 # RUN: echo "OUTPUT_FORMAT(x y z)" > %t8
 # RUN: not ld.lld %t8 no-such-file 2>&1 | FileCheck -check-prefix=ERR8 %s
 # ERR8: , expected, but got y
 # ERR8: cannot open no-such-file:

lld/trunk/test/ELF/linkerscript/linker-script-in-search-path.s

+# REQUIRES: x86
+# Check that we fall back to search paths if a linker script was not found
+# This behaviour matches ld.bfd and various projects appear to rely on this
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: mkdir -p %T/searchpath
+# RUN: echo "OUTPUT(\"%t.out\")" > %T/searchpath/foo.script
+# RUN: ld.lld -T%T/searchpath/foo.script %t.o
+# RUN: llvm-readobj %t.out | FileCheck %s
+# CHECK: Format: ELF64-x86-64
+
+# If the linker script specified with -T is missing we should emit an error
+# RUN: not ld.lld -Tfoo.script %t.o 2>&1 | FileCheck %s -check-prefix ERROR
+# ERROR: error: cannot find linker script foo.script
+
+# But if it exists in the search path we should fall back to that instead:
+# RUN: rm %t.out
+# RUN: ld.lld -L %T/searchpath -Tfoo.script %t.o
+# RUN: llvm-readobj %t.out | FileCheck %s

lld/trunk/test/ELF/linkerscript/linkerscript.s

 # RUN: echo "GROUP(\"%t\")" > %t.script2
 # RUN: ld.lld %t.script1
 # RUN: llvm-readobj %t2 > /dev/null
 
 # RUN: echo "INCLUDE \"foo.script\"" > %t.script
 # RUN: echo "OUTPUT(\"%t.out\")" > %T/foo.script
 # RUN: not ld.lld %t.script > %t.log 2>&1
 # RUN: FileCheck -check-prefix=INCLUDE_ERR %s < %t.log
-# INCLUDE_ERR: error: {{.+}}.script:1: cannot open foo.script
+# INCLUDE_ERR: error: {{.+}}.script:1: cannot find linker script foo.script
 # INCLUDE_ERR-NEXT: INCLUDE "foo.script"
 # RUN: ld.lld -L %T %t.script %t
 
 # RUN: echo "FOO(BAR)" > %t.script
 # RUN: not ld.lld -o foo %t.script > %t.log 2>&1
 # RUN: FileCheck -check-prefix=ERR1 %s < %t.log
 
 # ERR1: unknown directive: FOO
 
 .globl _start, _label
 _start:
   ret
 _label:
   ret