[ELF] - Allow emulation to be different from input objects target.

[grimar]

Summary

I found that when tried to link "arch/x86/realmode/rm" component of linux kernel,
LLD reports:

ld: arch/x86/realmode/rm/header.o is incompatible with elf_x86_64
...

All objects are i386, but invocation contains -m elf_x86_64 and we report a error.
That does not seem to be correct. Both gold and bfd allow to link that. Specification says:

-m emulation
              Emulate  the  emulation  linker.   You can list the
              available emulations with the --verbose or  -V  op-
              tions.   This  option overrides the compiled-in de-
              fault, which is the system for which you configured
              ld.

Patch makes target to be created basing on machine type inferred from
first object. If that is not possible then emulation option is used.

This is PR31678

[grimar]

• Removed dead code.

[ruiu]

If I understand correctly, you are trying to do this thing in this patch: set alternative values to page size, max page size and image base if -m is specified. Is this correct?

If so, I think this patch does that in a convoluted way.

[grimar]

In D28898#650644, @ruiu wrote:

If I understand correctly, you are trying to do this thing in this patch: set alternative values to page size, max page size and image base if -m is specified. Is this correct?

Main aim of that patch was to allow combination of -m elf_x86_64 and i386 objects.
Previously LLD took machine type and other data from -m and ignored inferring from objects and reported a error finally.
Patch changes logic to check objects first. And only after that check -m if not inferred data.

Setting alternative values is secondary thing here. Though it is looks consistent with what bfd do
(https://llvm.org/bugs/show_bug.cgi?id=31678) and documentation in general I think.

[grimar]

• Simplified. Changed behavior to just override the emulation with parameters taken from object.

(test/ELF/mips-n32-emul.s is removed)

[ruiu]

I don't think this patch works if all object files are in static archives.

[grimar]

In D28898#651577, @ruiu wrote:

I don't think this patch works if all object files are in static archives.

Original code also does not work when all files in static archives, no ? It requires -m or at least one .o file.

[ruiu]

In D28898#651623, @grimar wrote:

In D28898#651577, @ruiu wrote:

I don't think this patch works if all object files are in static archives.

Original code also does not work when all files in static archives, no ? It requires -m or at least one .o file.

I think you actually changed the behavior. Now it needs at least one .o even if -m is given.

ELF/Driver.cpp

702–703

I think you could just remove this piece of code from inferMachineTypes to do the same thing, no?

[grimar]

• Added testcase.

[grimar]

In D28898#651627, @ruiu wrote:

In D28898#651623, @grimar wrote:

In D28898#651577, @ruiu wrote:

I don't think this patch works if all object files are in static archives.

Original code also does not work when all files in static archives, no ? It requires -m or at least one .o file.

I think you actually changed the behavior. Now it needs at least one .o even if -m is given.

No, it does not need. I added a testcase for demonstration.

ELF/Driver.cpp

702–703

Since current logic is "assign parameters from -m and right after that override them with values from objects", I think it is better to localize this logic in a single place.

Its even may be worth some refactoring. I believe it is possible to get rid of that overriding. What we could to is check that -m is one of supported early. Then logic could be in different, correct order: first try to set machine type from files and then, if not successed - set it from -m. I had to use overriding only because we still want (I think) to check -m values even if it is ignored.

[ruiu]

Are you sure that this is the right thing to do? It seems alarming -- it doesn't feel right to me to handle the -m option like that. It doesn't make sense, and I cannot justify why it should be like that. Can you give me an explanation why this is correct?

[ruiu]

I found that arch/x86/realmode/rm/realmode.lds.S has an OUTPUT_FORMAT directive, which is ignored by LLD at the moment. Isn't this the cause of the problem?

[grimar]

In D28898#652502, @ruiu wrote:

I found that arch/x86/realmode/rm/realmode.lds.S has an OUTPUT_FORMAT directive, which is ignored by LLD at the moment. Isn't this the cause of the problem?

I think it is not. GNU linkers are very inconsistent. Ignoring OUTPUT_FORMAT seems to be OK.
See my experiments results:

Lets say we have next 4 scripts and trying to link i386 objects with -m elf_x86_64 (just like PR31678):
1) First that has OUTPUT_FORMAT and OUTPUT_ARCH

OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386")
OUTPUT_ARCH(i386)
SECTIONS { ... }

Linker: result
BFD: YES
GOLD: YES, output is i386 in both cases.

2) Second has only OUTPUT_FORMAT:

OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386")
SECTIONS { ... }

BFD: YES and output is x86_64. That looks wierd result for i386 objects inputs, right ?
And if I add --emit-relocs, I see that it emits tons of NONE relocations instead of i386 ones:

Relocation section '.rel.text' at offset 0x2061b8 contains 131 entries:
  Offset          Info           Type           Sym. Value    Sym. Name
000000000000  000000000000 R_X86_64_NONE    
000000000000  000000000000 R_X86_64_NONE    
000000000000  000000000000 R_X86_64_NONE

So it looks output is some kind of 'forced'.

GOLD: YES and output is i386.

3) Third has only OUTPUT_ARCH:

OUTPUT_ARCH(i386)
SECTIONS { ... }

BFD: NO
ld.bfd: i386 architecture of input file `home/umb/linux_kernel/linux/linux/arch/x86/realmode/rm/header.o' is incompatible with i386:x86-64 output

GOLD: YES and output is i386.

4) Fourth has nothing:

SECTIONS { ... }

BFD: NO

ld.bfd: i386 architecture of input file `home/umb/linux_kernel/linux/linux/arch/x86/realmode/rm/header.o' is incompatible with i386:x86-64 output

GOLD: YES, output is i386

So looks gold takes output format from inputs and ignores OUTPUT_FORMAT/OUTPUT_ARCH, just like we do.
Taking machine format from inputs and overriding -m seems very simple logic that is easy to understand and solves initial problem,
it is consistent with gold.
In my opinion gold behavior is much simpler than what bfd tries to do and we can try to stick to it.

[grimar]

• Updated in according to discussion.

[grimar]

ELF/Driver.cpp

722

This should be "all our inputs are binaries, "
At this point all bitcode files are already handled in loop above:

for (InputFile *F : Files) {

I'll update this comment for next diff/commit.

[ruiu]

What this does is probably okay, but this patch seems to be juggling data around unnecessarily. You probably want to restart from scratch to get cleaner code.

[ruiu]

I wouldn't submit this. Even two GNU linkers behave differently, and the new behavior this patch implements has odd semantics which is unable to describe other than "because the gold linker does that." And the original code that we are trying to handle seems to be just a typo. All in all, the best thing we should do now seems to fix the Linux kernel. If this issue resurfaces, we'll consider this patch again.

[grimar]

Bug was posted for linux kernel instead: https://bugzilla.kernel.org/show_bug.cgi?id=194091

ELF/Config.h

 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {
   InputFile *FirstElf = nullptr;
   uint8_t OSABI = 0;
   llvm::StringMap<uint64_t> SectionStartMap;
   llvm::StringRef DynamicLinker;
   llvm::StringRef Entry;
-  llvm::StringRef Emulation;
   llvm::StringRef Fini;
   llvm::StringRef Init;
   llvm::StringRef LTOAAPipeline;
   llvm::StringRef LTONewPmPasses;
   llvm::StringRef MapFile;
   llvm::StringRef OutputFile;
   llvm::StringRef SoName;
   llvm::StringRef Sysroot;
   llvm::StringSet<> RetainSymbolsFile;
+  std::pair<ELFKind, uint16_t> Emulation;
   std::string RPath;
   std::vector<VersionDefinition> VersionDefinitions;
   std::vector<llvm::StringRef> AuxiliaryList;
   std::vector<llvm::StringRef> SearchPaths;
   std::vector<llvm::StringRef> SymbolOrderingFile;
   std::vector<llvm::StringRef> Undefined;
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<SymbolVersion> VersionScriptLocals;

ELF/Driver.cpp

bool elf::link(ArrayRef<const char *> Args, bool CanExitEarly,
   ScriptConfig = make<ScriptConfiguration>();
 
   Driver->main(Args, CanExitEarly);
   freeArena();
   return !ErrorCount;
 }
 
 // Parses a linker -m option.
-static std::tuple<ELFKind, uint16_t, uint8_t> parseEmulation(StringRef Emul) {
-  uint8_t OSABI = 0;
+static void parseEmulation(StringRef Emul) {
+  if (Emul.empty())
+    return;
+
   StringRef S = Emul;
   if (S.endswith("_fbsd")) {
     S = S.drop_back(5);
-    OSABI = ELFOSABI_FREEBSD;
+    Config->OSABI = ELFOSABI_FREEBSD;
   }
 
-  std::pair<ELFKind, uint16_t> Ret =
+  Config->MipsN32Abi = Emul == "elf32btsmipn32" || Emul == "elf32ltsmipn32";
+
+  Config->Emulation =
       StringSwitch<std::pair<ELFKind, uint16_t>>(S)
           .Cases("aarch64elf", "aarch64linux", {ELF64LEKind, EM_AARCH64})
           .Case("armelf_linux_eabi", {ELF32LEKind, EM_ARM})
           .Case("elf32_x86_64", {ELF32LEKind, EM_X86_64})
           .Case("elf32btsmip", {ELF32BEKind, EM_MIPS})
           .Case("elf32ltsmip", {ELF32LEKind, EM_MIPS})
           .Case("elf32btsmipn32", {ELF32BEKind, EM_MIPS})
           .Case("elf32ltsmipn32", {ELF32LEKind, EM_MIPS})
           .Case("elf32ppc", {ELF32BEKind, EM_PPC})
           .Case("elf64btsmip", {ELF64BEKind, EM_MIPS})
           .Case("elf64ltsmip", {ELF64LEKind, EM_MIPS})
           .Case("elf64ppc", {ELF64BEKind, EM_PPC64})
           .Cases("elf_amd64", "elf_x86_64", {ELF64LEKind, EM_X86_64})
           .Case("elf_i386", {ELF32LEKind, EM_386})
           .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
           .Default({ELFNoneKind, EM_NONE});
 
-  if (Ret.first == ELFNoneKind) {
+  if (Config->Emulation.first == ELFNoneKind) {
     if (S == "i386pe" || S == "i386pep" || S == "thumb2pe")
       error("Windows targets are not supported on the ELF frontend: " + Emul);
     else
       error("unknown emulation: " + Emul);
   }
-  return std::make_tuple(Ret.first, Ret.second, OSABI);
 }
 
 // Returns slices of MB by parsing MB as an archive file.
 // Each slice consists of a member file in the archive.
 std::vector<MemoryBufferRef>
 LinkerDriver::getArchiveMembers(MemoryBufferRef MB) {
   std::unique_ptr<Archive> File =
       check(Archive::create(MB),
for (auto *Arg : Args.filtered(OPT_L))
     Config->SearchPaths.push_back(Arg->getValue());
 
   std::vector<StringRef> RPaths;
   for (auto *Arg : Args.filtered(OPT_rpath))
     RPaths.push_back(Arg->getValue());
   if (!RPaths.empty())
     Config->RPath = llvm::join(RPaths.begin(), RPaths.end(), ":");
 
-  if (auto *Arg = Args.getLastArg(OPT_m)) {
-    // Parse ELF{32,64}{LE,BE} and CPU type.
-    StringRef S = Arg->getValue();
-    std::tie(Config->EKind, Config->EMachine, Config->OSABI) =
-        parseEmulation(S);
-    Config->MipsN32Abi = (S == "elf32btsmipn32" || S == "elf32ltsmipn32");
-    Config->Emulation = S;
-  }
-
   Config->AllowMultipleDefinition = Args.hasArg(OPT_allow_multiple_definition);
   Config->Bsymbolic = Args.hasArg(OPT_Bsymbolic);
   Config->BsymbolicFunctions = Args.hasArg(OPT_Bsymbolic_functions);
   Config->Demangle = getArg(Args, OPT_demangle, OPT_no_demangle, true);
   Config->DisableVerify = Args.hasArg(OPT_disable_verify);
   Config->EhFrameHdr = Args.hasArg(OPT_eh_frame_hdr);
   Config->EnableNewDtags = !Args.hasArg(OPT_disable_new_dtags);
   Config->ExportDynamic =
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZWxneeded = hasZOption(Args, "wxneeded");
 
   Config->OFormatBinary = isOutputFormatBinary(Args);
   Config->SectionStartMap = getSectionStartMap(Args);
   Config->SortSection = getSortKind(Args);
   Config->Target2 = getTarget2Option(Args);
   Config->UnresolvedSymbols = getUnresolvedSymbolOption(Args);
 
+  parseEmulation(getString(Args, OPT_m));
+
   if (Args.hasArg(OPT_print_map))
     Config->MapFile = "-";
 
   // --omagic is an option to create old-fashioned executables in which
   // .text segments are writable. Today, the option is still in use to
   // create special-purpose programs such as boot loaders. It doesn't
   // make sense to create PT_GNU_RELRO for such executables.
   if (Config->OMagic)
case OPT_end_lib:
       break;
     }
   }
 
   if (Files.empty() && ErrorCount == 0)
     error("no input files");
 }
 
-// If -m <machine_type> was not given, infer it from object files.
 void LinkerDriver::inferMachineType() {
-  if (Config->EKind != ELFNoneKind)
-    return;
-
+  // We want to try infer machine type from inputs.
+  // If we have a success, -m option is ignored (except OSABI value).
   for (InputFile *F : Files) {
     if (F->EKind == ELFNoneKind)
       continue;
     Config->EKind = F->EKind;
     Config->EMachine = F->EMachine;
-    Config->OSABI = F->OSABI;
     Config->MipsN32Abi = Config->EMachine == EM_MIPS && isMipsN32Abi(F);
+    // But if we already have ABI set, we keep it. That allows to keep
+    // ELFOSABI_FREEBSD flag set from -m.
+    if (!Config->OSABI)
+      Config->OSABI = F->OSABI;
     return;
   }
-  error("target emulation unknown: -m or at least one .o file required");
+
+  // If we have at least one archive, -m is ignored and we require
+  // to have at least one ELF input file.
+  bool HasArchive = llvm::find_if(Files, [](InputFile *F) {
+                      return F->kind() == InputFile::ArchiveKind;
+                    }) != Files.end();
+  if (HasArchive) {
+    error("target unknown: at least one ELF file required");
+    return;
+  }
+
+  // If all our inputs are binaries or bitcode files,
+  // and we have no known machine type at this point,
+  // apply the emulation.
+  Config->EKind = Config->Emulation.first;
+  Config->EMachine = Config->Emulation.second;
+  if (Config->EKind == ELFNoneKind)
+    error("target emulation unknown: -m is required");
 }
 
 // Parse -z max-page-size=<value>. The default value is defined by
 // each target.
 static uint64_t getMaxPageSize(opt::InputArgList &Args) {
   uint64_t Val =
       getZOptionValue(Args, "max-page-size", Target->DefaultMaxPageSize);
   if (!isPowerOf2_64(Val))

ELF/SymbolTable.cpp

template <class ELFT> static bool isCompatible(InputFile *F) {
 
   if (F->EKind == Config->EKind && F->EMachine == Config->EMachine) {
     if (Config->EMachine != EM_MIPS)
       return true;
     if (isMipsN32Abi(F) == Config->MipsN32Abi)
       return true;
   }
 
-  if (!Config->Emulation.empty())
-    error(toString(F) + " is incompatible with " + Config->Emulation);
-  else
-    error(toString(F) + " is incompatible with " + toString(Config->FirstElf));
   return false;
 }
 
 // Add symbols in File to the symbol table.
 template <class ELFT> void SymbolTable<ELFT>::addFile(InputFile *File) {
   if (!isCompatible<ELFT>(File))
     return;
 

test/ELF/emulation-archive.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+# RUN: llvm-ar rcs %tar %t
+
+# RUN: not ld.lld %tar -o %tout 2>&1 | FileCheck %s
+# CHECK: target unknown: at least one ELF file required
+
+# RUN: not ld.lld -m elf_x86_64 %tar -o %tout 2>&1 | FileCheck %s

test/ELF/emulation-override.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=i686-unknown-linux %s -o %t.o
+
+## Show target output without emulation.
+# RUN: ld.lld %t.o -o %t1
+# RUN: llvm-readobj %t1 | FileCheck %s
+# CHECK:      Format: ELF32-i386
+# CHECK-NEXT: Arch: i386
+
+## Check that we are able to apply elf_x86_64 emulation for i386 object.
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+
+.globl _start
+_start:
+ nop

test/ELF/incompatible.s

 // RUN: not ld.lld %tc.o %ti686.so -o %t 2>&1 | \
 // RUN:   FileCheck --check-prefix=C-AND-SO %s
 // C-AND-SO: i686.so is incompatible with {{.*}}c.o
 
 // RUN: not ld.lld %ti686.so %tc.o -o %t 2>&1 | \
 // RUN:   FileCheck --check-prefix=SO-AND-C %s
 // SO-AND-C: c.o is incompatible with {{.*}}i686.so
 
-// RUN: not ld.lld -m elf64ppc %ta.o -o %t 2>&1 | \
-// RUN:   FileCheck --check-prefix=A-ONLY %s
-// A-ONLY: a.o is incompatible with elf64ppc
-
-// RUN: not ld.lld -m elf64ppc %tb.o -o %t 2>&1 | \
-// RUN:   FileCheck --check-prefix=B-ONLY %s
-// B-ONLY: b.o is incompatible with elf64ppc
-
-// RUN: not ld.lld -m elf64ppc %tc.o -o %t 2>&1 | \
-// RUN:   FileCheck --check-prefix=C-ONLY %s
-// C-ONLY: c.o is incompatible with elf64ppc
-
-// RUN: not ld.lld -m elf_i386 %tc.o %ti686.so -o %t 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 %t 2>&1 | \
-// RUN:   FileCheck --check-prefix=SO-AND-C-I386 %s
-// SO-AND-C-I386: c.o is incompatible with elf_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: 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 | FileCheck --check-prefix=ARCHIVE %s
 // ARCHIVE: .a({{.*}}a.o) is incompatible with {{.*}}b.o
 .global _start
 _start:
 .data
         .long foo
 
 // REQUIRES: x86,aarch64

test/ELF/mips-n32-emul.s

-# Check that LLD shows an error when N32 ABI emulation argument
-# is combined with non-N32 ABI object files.
-
-# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
-# RUN: not ld.lld -m elf32btsmipn32 %t.o -o %t.exe 2>&1 | FileCheck %s
-
-# REQUIRES: mips
-
-  .text
-  .global  __start
-__start:
-  nop
-
-# CHECK: error: {{.*}}mips-n32-emul.s.tmp.o is incompatible with elf32btsmipn32

test/ELF/no-obj.s

 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-ar rcs %t.a %t.o
 // RUN: not ld.lld -o %t2 -u _start %t.a 2>&1 | FileCheck %s
 
-// CHECK: target emulation unknown: -m or at least one .o file required
+// CHECK: target unknown: at least one ELF file required
 
 .global _start
 _start: