Add support for OUTPUT_ARCH linker script command

[void]

Summary

The OUTPUT_ARCH linker script command overrides the "-m <arch>"
command line flag.

[void]

Note: I tried to add as many BFD arch entries as I could muster from binutils. It's obvious not complete, so any further entries you think should be here let me know.

[grimar]

What is your use case? I wonder what is the reason to use OUTPUT_ARCH to override -m?

We started to support OUTPUT_FORMAT recently, but it does not override the
EKind, EMachine and MipsN32Abi values if they were already found from reading the files.

ELF/ScriptParser.cpp

407

I did not debug it, but I think the following case would fail:

If you take 32bit object test32.o and a script saying the arch is 64,
and invoke ld.lld test32.o -T script

then the script should override the EKind/EMachine and crash the linker somewhere
(I suspect it will try to parse the files using a wrong ELFT then).

[ruiu]

What is the motivation to do this? We don't try too hard to implement every detail of the linker script because it's just too complicated and there is usually an easy workaround that works without a linker script.

[void]

In D55311#1319943, @grimar wrote:

What is your use case? I wonder what is the reason to use OUTPUT_ARCH to override -m?

It was pointed out that the "-m" flag has slightly different semantics than what we would expect:

From Ian Taylor:

"The documentation for the option says:

-m emulation
    Emulate the emulation linker. You can list the available emulations with the ‘--verbose’ or ‘-V’ options.
    If the ‘-m’ option is not used, the emulation is taken from the LDEMULATION environment variable, if that is defined.
    Otherwise, the default emulation depends upon how the linker was configured.

"The point is that the emulation does not primarily set the expected object file format. It sets the default linker behavior. With the BFD linker, it primarily sets the linker script to use (the BFD linker always uses a linker script). If you then provide a different linker script using the -T option, that naturally overrides the linker script selected by the -m option."

[void]

In D55311#1320159, @ruiu wrote:

What is the motivation to do this? We don't try too hard to implement every detail of the linker script because it's just too complicated and there is usually an easy workaround that works without a linker script.

The motivation is to support linking Linux, which uses linker scripts heavily. I added a comment by IanT from another thread about why this is a good addition.

[void]

ELF/ScriptParser.cpp

407

It doesn't look like that's the case:

[morbo@fawn:llvm] cat script 
OUTPUT_ARCH(i386:x86-64)
[morbo@fawn:llvm] clang -m32 -c z.c
[morbo@fawn:llvm] ./llvm.opt.obj/bin/ld.lld z.o -T script 
ld.lld: warning: cannot find entry symbol _start; defaulting to 0x401000

[ruiu]

Even with this patch, don't you still have to make a change to lld so that it accepts a set of input files of different targets?

[void]

In D55311#1320699, @ruiu wrote:

Even with this patch, don't you still have to make a change to lld so that it accepts a set of input files of different targets?

I don't understand. They should all be for the same target. This is just telling the linker which target they're actually for.

[ruiu]

Hmm, you said that you are handling inputs that are mix of i386 and x86-64 object files, so I thought this is for that issue. What am I missing?

[ruiu]

Looks like what you are trying to do is to generate a x86-64 executable from i386 object files. That's what we do not expect in lld. With this patch, in theory you can create AArch64 exectuables from x86-64 object files, but that doesn't make sense and likely to crash the linker because we assume that all object files are uniform in terms of target types.

I think this use case is too tricky to directly support in the linker. I'd probably use objcopy or something to transplant i386 code to x86-64 object file before passing it to the linker, so that we don't deal with the trickiness in the linker.

[grimar]

In D55311#1320583, @void wrote:

In D55311#1320159, @ruiu wrote:

What is the motivation to do this? We don't try too hard to implement every detail of the linker script because it's just too complicated and there is usually an easy workaround that works without a linker script.

The motivation is to support linking Linux, which uses linker scripts heavily. I added a comment by IanT from another thread about why this is a good addition.

Now I remember. We saw this bug earlier and decided to report an issue to linux kernel and not change the linker.
Bug reported here: https://bugzilla.kernel.org/show_bug.cgi?id=194091.

[void]

In D55311#1320721, @ruiu wrote:

Looks like what you are trying to do is to generate a x86-64 executable from i386 object files. That's what we do not expect in lld. With this patch, in theory you can create AArch64 exectuables from x86-64 object files, but that doesn't make sense and likely to crash the linker because we assume that all object files are uniform in terms of target types.

I think this use case is too tricky to directly support in the linker. I'd probably use objcopy or something to transplant i386 code to x86-64 object file before passing it to the linker, so that we don't deal with the trickiness in the linker.

Sorry, I've never said that I want to generate an x86-64 executable from i386 object files. The OUTPUT_ARCH command doesn't do that anyway...

[void]

In D55311#1320722, @grimar wrote:

In D55311#1320583, @void wrote:

In D55311#1320159, @ruiu wrote:

What is the motivation to do this? We don't try too hard to implement every detail of the linker script because it's just too complicated and there is usually an easy workaround that works without a linker script.

The motivation is to support linking Linux, which uses linker scripts heavily. I added a comment by IanT from another thread about why this is a good addition.

Now I remember. We saw this bug earlier and decided to report an issue to linux kernel and not change the linker.
Bug reported here: https://bugzilla.kernel.org/show_bug.cgi?id=194091.

That bug report is very old and no action has been taken on it. From what I copied above from Ian Taylor, the bug you reported isn't actually a bug, but a misunderstanding of what the -m flag means.

[ruiu]

Sorry, I'm really confused. Could you explain again for me why you need this patch to link Linux kernel?

[void]

In D55311#1320727, @ruiu wrote:

Sorry, I'm really confused. Could you explain again for me why you need this patch to link Linux kernel?

The Linux kernel uses linker scripts heavily. They include the OUTPUT_ARCH command. It's a feature that's not quite the same as the -m command line option. It's not cruft and has an actual real use.

ELF/ScriptParser.cpp

private:
   InputSectionDescription *readInputSectionDescription(StringRef Tok);
   StringMatcher readFilePatterns();
   std::vector<SectionPattern> readInputSectionsList();
   InputSectionDescription *readInputSectionRules(StringRef FilePattern);
   unsigned readPhdrType();
   SortSectionPolicy readSortKind();
   SymbolAssignment *readProvideHidden(bool Provide, bool Hidden);
   SymbolAssignment *readAssignment(StringRef Tok);
+  std::pair<ELFKind, uint16_t> readBfdArch();
   std::tuple<ELFKind, uint16_t, bool> readBfdName();
   void readSort();
   Expr readAssert();
   Expr readConstant();
   Expr getPageSize();
 
   uint64_t readMemoryAssignment(StringRef, StringRef, StringRef);
   std::pair<uint32_t, uint32_t> readMemoryAttributes();
void ScriptParser::readOutput() {
   // -o <file> takes predecence over OUTPUT(<file>).
   expect("(");
   StringRef Tok = next();
   if (Config->OutputFile.empty())
     Config->OutputFile = unquote(Tok);
   expect(")");
 }
 
+std::pair<ELFKind, uint16_t> ScriptParser::readBfdArch() {
+  StringRef S = next();
+
+  std::pair<ELFKind, uint16_t> Arch =
+      StringSwitch<std::pair<ELFKind, uint16_t>>(S)
+          .Cases("aarch64", "aarch64:ilp32", {ELF64LEKind, EM_AARCH64})
+          .Case("arm", {ELF32LEKind, EM_ARM})
+          .Cases("i386", "i386:intel", "i386:x64-32:intel", "i8086",
+                 "i386:x64-32", "i386:x64-32:nacl", "i386:nacl",
+                 {ELF32LEKind, EM_386})
+          .Cases("i386:x86-64", "i386:x64-64:intel", "i386:x86-64:nacl",
+                 {ELF64LEKind, EM_X86_64})
+          .Case("powerpc:common", {ELF32BEKind, EM_PPC})
+          .Case("powerpc:common64", {ELF64BEKind, EM_PPC64})
+          .Default({ELFNoneKind, EM_NONE});
+
+  if (Arch.first == ELFNoneKind)
+    setError("unknown output arch name: " + S);
+
+  return Arch;
+}
+
 void ScriptParser::readOutputArch() {
-  // OUTPUT_ARCH is ignored for now.
+  // This overrides the "-m <arch>" flag.
   expect("(");
-  while (!errorCount() && !consume(")"))
-    skip();
+  std::tie(Config->EKind, Config->EMachine) = readBfdArch();
+  expect(")");
 }
 
 std::tuple<ELFKind, uint16_t, bool> ScriptParser::readBfdName() {
   StringRef S = unquote(next());
   if (S == "elf32-i386")
     return std::make_tuple(ELF32LEKind, EM_386, false);
   if (S == "elf32-iamcu")
     return std::make_tuple(ELF32LEKind, EM_IAMCU, false);

test/ELF/linkerscript/outputarch.test

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd /dev/null -o %t1
 # RUN: ld.lld -shared -o %t2 %t1 %s
 
-OUTPUT_ARCH(All data written here is ignored)
+OUTPUT_ARCH(i386:x86-64)