[lld] 2822852 - [ELF] Correct error message when OUTPUT_FORMAT is used

[Shoaib Meenai via llvm-commits]

Author: Shoaib Meenai
Date: 2020-03-12T22:54:53-07:00
New Revision: 2822852ffc4649b09670e6f287871990536e3c7b

URL: https://github.com/llvm/llvm-project/commit/2822852ffc4649b09670e6f287871990536e3c7b
DIFF: https://github.com/llvm/llvm-project/commit/2822852ffc4649b09670e6f287871990536e3c7b.diff

LOG: [ELF] Correct error message when OUTPUT_FORMAT is used

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.

Differential Revision: https://reviews.llvm.org/D76109

Added: 
    

Modified: 
    lld/ELF/Config.h
    lld/ELF/InputFiles.cpp
    lld/ELF/ScriptParser.cpp
    lld/test/ELF/incompatible.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 05f44a5fb7aa..9f9f0d696f63 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -90,6 +90,7 @@ struct Configuration {
   uint32_t andFeatures = 0;
   llvm::CachePruningPolicy thinLTOCachePolicy;
   llvm::StringMap<uint64_t> sectionStartMap;
+  llvm::StringRef bfdname;
   llvm::StringRef chroot;
   llvm::StringRef dynamicLinker;
   llvm::StringRef dwoDir;

diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 1d8a20aa1388..8c7ea91e05f0 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -138,8 +138,10 @@ static bool isCompatible(InputFile *file) {
       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;
   }
 
@@ -148,8 +150,11 @@ static bool isCompatible(InputFile *file) {
     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;

diff  --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index 11e87197a9a2..0a041202f278 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -412,14 +412,14 @@ static std::pair<ELFKind, uint16_t> parseBfdName(StringRef s) {
 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;
 

diff  --git a/lld/test/ELF/incompatible.s b/lld/test/ELF/incompatible.s
index b7e784a0657c..9a52c40eab67 100644
--- a/lld/test/ELF/incompatible.s
+++ b/lld/test/ELF/incompatible.s
@@ -44,6 +44,14 @@
 // 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.