[ELF] Correct error message when OUTPUT_FORMAT is used

[smeenai]

Summary

Any OUTPUT_FORMAT in a linker script overrides the emulation passed on
the command line, so record the passed bfdname and use that in the error
message about incompatible input files.

This prevents confusing error messages. For example, if you explicitly
pass -m elf_x86_64 to LLD but accidentally include a linker script
which sets OUTPUT_FORMAT(elf32-i386), LLD would previously complain
about your input files being compatible with elf_x86_64, which isn't the
actual issue, and is confusing because the input files are in fact
x86-64 ELF files.

Interestingly enough, this also prevents a segfault! When we don't pass
-m and we have an object file which is incompatible with the
OUTPUT_FORMAT set by a linker script, the object file is checked for
compatibility before it's added to the objectFiles vector.
config->emulation, objectFiles, and sharedFiles will all be empty, so
we'll attempt to access bitcodeFiles[0], but bitcodeFiles is also empty,
so we'll segfault. This commit prevents the segfault by adding
OUTPUT_FORMAT as a possible source of machine configuration, and it also
adds an llvm_unreachable to diagnose similar issues in the future.

[MaskRay]

The logic is good.

lld/ELF/ScriptParser.cpp

414–415

The local variable can be omitted now.

[smeenai]

lld/ELF/ScriptParser.cpp

414–415

It's also used in line 423 below, but I'm happy to change all uses.

[smeenai]

Eliminate local variable

[ruiu]

LGTM

[grimar]

lld/ELF/ScriptParser.cpp

422

I think we might want to have a test that uses something like 'foobar-freebsd'
and checks that we print foobar-freebsd and not just foobar contained in s.

[smeenai]

lld/ELF/ScriptParser.cpp

422

Great suggestion. Done in https://github.com/llvm/llvm-project/commit/e890453d6d11

[grimar]

lld/ELF/ScriptParser.cpp

422

Done in https://github.com/llvm/llvm-project/commit/e890453d6d11

Thanks!

lld/ELF/Config.h

 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {
   uint8_t osabi = 0;
   uint32_t andFeatures = 0;
   llvm::CachePruningPolicy thinLTOCachePolicy;
   llvm::StringMap<uint64_t> sectionStartMap;
+  llvm::StringRef bfdname;
   llvm::StringRef chroot;
   llvm::StringRef dynamicLinker;
   llvm::StringRef dwoDir;
   llvm::StringRef entry;
   llvm::StringRef emulation;
   llvm::StringRef fini;
   llvm::StringRef init;
   llvm::StringRef ltoAAPipeline;

lld/ELF/InputFiles.cpp

static bool isCompatible(InputFile *file) {
 
   if (file->ekind == config->ekind && file->emachine == config->emachine) {
     if (config->emachine != EM_MIPS)
       return true;
     if (isMipsN32Abi(file) == config->mipsN32Abi)
       return true;
   }
 
-  if (!config->emulation.empty()) {
-    error(toString(file) + " is incompatible with " + config->emulation);
+  StringRef target =
+      !config->bfdname.empty() ? config->bfdname : config->emulation;
+  if (!target.empty()) {
+    error(toString(file) + " is incompatible with " + target);
     return false;
   }
 
   InputFile *existing;
   if (!objectFiles.empty())
     existing = objectFiles[0];
   else if (!sharedFiles.empty())
     existing = sharedFiles[0];
-  else
+  else if (!bitcodeFiles.empty())
     existing = bitcodeFiles[0];
+  else
+    llvm_unreachable("Must have -m, OUTPUT_FORMAT or existing input file to "
+                     "determine target emulation");
 
   error(toString(file) + " is incompatible with " + toString(existing));
   return false;
 }
 
 template <class ELFT> static void doParseFile(InputFile *file) {
   if (!isCompatible(file))
     return;

lld/ELF/ScriptParser.cpp

return StringSwitch<std::pair<ELFKind, uint16_t>>(s)
       .Case("elf64-littleriscv", {ELF64LEKind, EM_RISCV})
       .Default({ELFNoneKind, EM_NONE});
 }
 
 // Parse OUTPUT_FORMAT(bfdname) or OUTPUT_FORMAT(bfdname, big, little).
 // Currently we ignore big and little parameters.
 void ScriptParser::readOutputFormat() {
   expect("(");
 
-  StringRef name = unquote(next());
-  StringRef s = name;
+  config->bfdname = unquote(next());
+  StringRef s = config->bfdname;
   if (s.consume_back("-freebsd"))
     config->osabi = ELFOSABI_FREEBSD;
 
   std::tie(config->ekind, config->emachine) = parseBfdName(s);
   if (config->emachine == EM_NONE)
-    setError("unknown output format name: " + name);
+    setError("unknown output format name: " + config->bfdname);
   if (s == "elf32-ntradlittlemips" || s == "elf32-ntradbigmips")
     config->mipsN32Abi = true;
 
   if (consume(")"))
     return;
   expect(",");
   skip();
   expect(",");

lld/test/ELF/incompatible.s

 // RUN: not ld.lld -m elf_i386 %tc.o %ti686.so -o /dev/null 2>&1 | \
 // RUN:   FileCheck --check-prefix=C-AND-SO-I386 %s
 // C-AND-SO-I386: c.o is incompatible with elf_i386
 
 // RUN: not ld.lld -m elf_i386 %ti686.so %tc.o -o /dev/null 2>&1 | \
 // RUN:   FileCheck --check-prefix=SO-AND-C-I386 %s
 // SO-AND-C-I386: c.o is incompatible with elf_i386
 
+// RUN: echo 'OUTPUT_FORMAT(elf32-i386)' > %t.script
+// RUN: not ld.lld %t.script %ta.o -o /dev/null 2>&1 | \
+// RUN:   FileCheck --check-prefix=A-AND-SCRIPT %s
+// RUN: not ld.lld %ta.o %t.script -o /dev/null 2>&1 | \
+// RUN:   FileCheck --check-prefix=A-AND-SCRIPT %s
+// RUN: not ld.lld -m elf_x86_64 %ta.o %t.script -o /dev/null 2>&1 | \
+// RUN:   FileCheck --check-prefix=A-AND-SCRIPT %s
+// A-AND-SCRIPT: a.o is incompatible with elf32-i386
 
 // We used to fail to identify this incompatibility and crash trying to
 // read a 64 bit file as a 32 bit one.
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/archive2.s -o %ta.o
 // RUN: rm -f %t.a
 // RUN: llvm-ar rc %t.a %ta.o
 // RUN: llvm-mc -filetype=obj -triple=i686-linux %s -o %tb.o
 // RUN: not ld.lld %t.a %tb.o 2>&1 -o /dev/null | FileCheck --check-prefix=ARCHIVE %s
 // ARCHIVE: .a({{.*}}a.o) is incompatible with {{.*}}b.o
 .global _start
 _start:
 .data
         .long foo