[lld][ELF] Add support for -b binary

[Bigcheese]

Summary

Implemented by building an ELF file in memory.

The -b formats 'elf', 'default', and 'binary' match gold behavior.

This is needed for a bunch of kernel drivers in FreeBSD.

[ruiu]

ELF/Driver.cpp

500

I'd directly call readLinkerScript if OPT_script. It should allows you to remove the second parameter from addFile.

506

We use S as a local variable name in other places for a return value of getValue.

515

I think getSpelling always return -b, so I'd just print out "unknown -b format: ".

ELF/InputFiles.cpp

714

Can you move all these details into SimpleELFWriter so that we can just do something like

std::vector<uint8_t> Buf = createELFFromBinary(MB);
return createELFFile<ObjectFile>(MemoryBufferRef(Buf, MB.getBufferIdentifier()));

ELF/SimpleELFWriter.h

1 ↗

(On Diff #69771)

Writer is I think a confusing name because it sounds like an alternative Writer. It is actually rather a part of readers. I'd probably name this BinaryFile.h.

[Bigcheese]

ELF/Driver.cpp

500

I was looking at that, but then I'd have to copy:

if (Config->Verbose)
  outs() << Path << "\n";

Optional<MemoryBufferRef> Buffer = readFile(Path);
if (!Buffer.hasValue())
  return;
MemoryBufferRef MBRef = *Buffer;

Which seems like more complexity than adding a param.

506

Will do.

515

It can also return -format=.

Also I believe that would be bad practice in general. It makes it really easy to get into the situation where we print the "wrong" thing.

ELF/InputFiles.cpp

714

If the concern is just having this much code in InputFile.cpp I'm fine with moving it somewhere else and calling it from here.

As for moving the code into SimpleELFWriter, I would rather keep SimpleELFWriter simple and not mix responsibilities. I plan to extend/refactor SimpleELFWriter to LLVMObject at some point and use it for final ELF emission for MC, split dwarf, objcopy (maybe), etc.

This was extracted (and reduced) from our internal version of the linker which has need of it elsewhere. So I'm already at 3 active uses of this code including this patch.

[ruiu]

ELF/Driver.cpp

500

Ah right. I'm fine with your change.

ELF/InputFiles.cpp

714

My concern was mostly about the amount of code you added to this file. What we want to do here is to wrap a binary blob with an ELF header, and we are not interested in the details, so I wanted to move all these details to other file.

I see a room to simplify the writer thing, but as long as it is encapsulated into one file, we can improve the implementation independently from other file.

ELF/SimpleELFWriter.h

31 ↗

(On Diff #69771)

Please create a .cpp file and move the definitions to that file.

[• rafael]

ELF/InputFiles.cpp

724

These names look wrong. With gold and bfd I get

_binaryhome_espindola_llvm_build_tools_lld_test_ELF_Output_format_binary_test_tmp_binary_start
_binaryhome_espindola_llvm_build_tools_lld_test_ELF_Output_format_binary_test_tmp_binary_size
_binary__home_espindola_llvm_build_tools_lld_test_ELF_Output_format_binary_test_tmp_binary_end

[emaste]

ELF/InputFiles.cpp

724

I believe it should be _binary_ + alnum_pathname + {_start, _end, _size}, where alnum_pathname is the full pathname passed to the linker with non-alnum characters replaced by _.

[• rafael]

I still thing that writing an ELF to read it back is a bad design, but the refactoring needed to avoid it is non-trivial. We are almost there for symbols, but sections require more work.

So please upload a last version with the linux build fixed, the symbol names changes and the nit in this comment. With that, it probably LGTM and will try to refactor it afterwards.

ELF/Driver.cpp

507

You can write this as

case OPT_format: {
  StringRef Val = Arg->getValue();
  if (Val == "elf" || Val == "default")
    Config->Binary = false;
  else if (Val == "binary")
    Config->Binary = true;
  else
    error("unknown " + Arg->getSpelling() + " format: " + Arg->getValue() +
          " (supported formats: elf, default, binary)");
  break;
}

[Bigcheese]

• Split SimpleELFWriter into .cpp
• Fix nits
• Fix linux build
• Fix symbol names

[• rafael]

LGTM

test/elf/format-binary.test

2

The test directory name is wrong. It should be ELF, not elf.

[emaste]

One issue remains in using this for the FreeBSD kernel module build -- we don't specify the output format / emulation right now, because we build GNU ld with the output target compiled-in. This results in an error (see D24348 for my suggested improvement to the error message).

I think it's much better if we don't have to build the linker multiple times though (once for each supported target). The right thing for us to do is make the build pass in -m.

[ruiu]

ELF/CMakeLists.txt

20

I still don't like this file name. SimpleELFWriter sounds like it is for -o binary instead of -b. Can you rename BinaryFile.cpp?

ELF/InputFiles.cpp

736

Please move these details to SimpleELFWriter.cpp.

[emaste]

The right thing for us to do is make the build pass in -m.

That change is now in review for FreeBSD, https://reviews.freebsd.org/D7837

[Bigcheese]

• Change name to ELFCreator

[ruiu]

ELF/InputFiles.cpp

736

How about this?

[Bigcheese]

ELF/InputFiles.cpp

736

Well, the code doesn't belong in ELFCreator.cpp. I can make a BinaryFile.cpp for it, but it's only 40 lines of code.

[ruiu]

ELF/InputFiles.cpp

736

That distinction doesn't make much sense to me because this is part of the linker. If you have a concrete plan to move ELFCreator to some library, it may make sense, but it doesn't seem to happen soon.

[silvas]

ELF/InputFiles.cpp

736

Like Michael already said, ELFCreator is already used in at least 2 other places for PS4. It doesn't make sense to move this BinaryFile-specific code into ELFCreator.

[ruiu]

ELF/InputFiles.h

310

Is this equivalent to std::unique_ptr<char *>? If so, please write that way.

Can you use uint8_t instead of char?

[Bigcheese]

ELF/InputFiles.h

310

It needs to be [] so that delete[] is called instead of delete. But I'm actually going to change it to std::vector.

I can change it to uint8_t.

[ruiu]

LGTM. Let's land this. Thank you for doing this!

ELF/CMakeLists.txt

   Driver.cpp
   DriverUtils.cpp
   EhFrame.cpp
+  ELFCreator.cpp
   Error.cpp
   ICF.cpp
   InputFiles.cpp

ELF/Config.h

   std::vector<uint8_t> BuildIdVector;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
+  bool Binary = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;
   bool Demangle = true;

ELF/Driver.h

 class LinkerDriver {
 public:
   void main(ArrayRef<const char *> Args);
-  void addFile(StringRef Path);
+  void addFile(StringRef Path, bool KnownScript = false);
   void addLibrary(StringRef Name);
   llvm::LLVMContext Context;      // to parse bitcode ifles
   std::unique_ptr<CpioFile> Cpio; // for reproduce

ELF/Driver.cpp

 
 // Opens and parses a file. Path has to be resolved already.
 // Newly created memory buffers are owned by this driver.
-void LinkerDriver::addFile(StringRef Path) {
+void LinkerDriver::addFile(StringRef Path, bool KnownScript) {
   using namespace sys::fs;
   if (Config->Verbose)
     outs() << Path << "\n";

     return;
   MemoryBufferRef MBRef = *Buffer;
 
+  if (Config->Binary && !KnownScript) {
+    Files.push_back(make_unique<BinaryFile>(MBRef));
+    return;
+  }
+
   switch (identify_magic(MBRef.getBuffer())) {
   case file_magic::unknown:
     readLinkerScript(MBRef);

     case OPT_l:
       addLibrary(Arg->getValue());
       break;
-    case OPT_alias_script_T:
     case OPT_INPUT:
-    case OPT_script:
       addFile(Arg->getValue());
       break;
+    case OPT_alias_script_T:
+    case OPT_script:
+      addFile(Arg->getValue(), true);
+      break;
     case OPT_as_needed:
       Config->AsNeeded = true;
       break;
+    case OPT_format: {
+      StringRef Val = Arg->getValue();
+      if (Val == "elf" || Val == "default")
+        Config->Binary = false;
+      else if (Val == "binary")
+        Config->Binary = true;
+      else
+        error("unknown " + Arg->getSpelling() + " format: " + Arg->getValue() +
+              " (supported formats: elf, default, binary)");
+      break;
+    }
     case OPT_no_as_needed:
       Config->AsNeeded = false;
       break;

ELF/ELFCreator.h

+//===- ELFCreator.h ---------------------------------------------*- C++ -*-===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_ELF_ELF_CREATOR_H
+#define LLD_ELF_ELF_CREATOR_H
+
+#include "lld/Core/LLVM.h"
+
+#include "llvm/MC/StringTableBuilder.h"
+#include "llvm/Object/ELFTypes.h"
+
+namespace lld {
+namespace elf {
+
+template <class ELFT> class ELFCreator {
+  typedef typename ELFT::uint uintX_t;
+  typedef typename ELFT::Ehdr Elf_Ehdr;
+  typedef typename ELFT::Shdr Elf_Shdr;
+  typedef typename ELFT::Sym Elf_Sym;
+
+public:
+  struct Section {
+    Elf_Shdr *Header;
+    std::size_t Index;
+  };
+
+  struct Symbol {
+    Elf_Sym *Sym;
+    std::size_t Index;
+  };
+
+  ELFCreator(std::uint16_t Type, std::uint16_t Machine);
+  Section addSection(StringRef Name);
+  Symbol addSymbol(StringRef Name);
+  std::size_t layout();
+  void write(char *Out);
+
+private:
+  Elf_Ehdr Header;
+  std::vector<Elf_Shdr *> Sections;
+  std::vector<Elf_Sym *> StaticSymbols;
+  llvm::StringTableBuilder SecHdrStrTabBuilder{llvm::StringTableBuilder::ELF};
+  llvm::StringTableBuilder StrTabBuilder{llvm::StringTableBuilder::ELF};
+  llvm::BumpPtrAllocator Alloc;
+  Elf_Shdr *ShStrTab;
+  Elf_Shdr *StrTab;
+  Elf_Shdr *SymTab;
+};
+}
+}
+
+#endif

ELF/ELFCreator.cpp

+//===- ELFCreator.cpp -----------------------------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ELFCreator.h"
+
+using namespace llvm;
+using namespace llvm::ELF;
+using namespace llvm::object;
+
+using namespace lld;
+using namespace lld::elf;
+
+template <class ELFT>
+ELFCreator<ELFT>::ELFCreator(std::uint16_t Type, std::uint16_t Machine) {
+  std::memcpy(Header.e_ident, "\177ELF", 4);
+  Header.e_ident[EI_CLASS] = ELFT::Is64Bits ? ELFCLASS64 : ELFCLASS32;
+  Header.e_ident[EI_DATA] = ELFT::TargetEndianness == llvm::support::little
+                                ? ELFDATA2LSB
+                                : ELFDATA2MSB;
+  Header.e_ident[EI_VERSION] = EV_CURRENT;
+  Header.e_ident[EI_OSABI] = 0;
+  Header.e_type = Type;
+  Header.e_machine = Machine;
+  Header.e_version = EV_CURRENT;
+  Header.e_entry = 0;
+  Header.e_phoff = 0;
+  Header.e_flags = 0;
+  Header.e_ehsize = sizeof(Elf_Ehdr);
+  Header.e_phnum = 0;
+  Header.e_shentsize = sizeof(Elf_Shdr);
+  Header.e_shstrndx = 1;
+
+  ShStrTab = addSection(".shstrtab").Header;
+  ShStrTab->sh_type = SHT_STRTAB;
+  ShStrTab->sh_addralign = 1;
+
+  StrTab = addSection(".strtab").Header;
+  StrTab->sh_type = SHT_STRTAB;
+  StrTab->sh_addralign = 1;
+
+  SymTab = addSection(".symtab").Header;
+  SymTab->sh_type = SHT_SYMTAB;
+  SymTab->sh_link = 2;
+  SymTab->sh_info = 1;
+  SymTab->sh_addralign = sizeof(uintX_t);
+  SymTab->sh_entsize = sizeof(Elf_Sym);
+}
+
+template <class ELFT>
+typename ELFCreator<ELFT>::Section
+ELFCreator<ELFT>::addSection(StringRef Name) {
+  std::size_t NameOff = SecHdrStrTabBuilder.add(Name);
+  auto Shdr = new (Alloc) Elf_Shdr{};
+  Shdr->sh_name = NameOff;
+  Sections.push_back(Shdr);
+  return {Shdr, Sections.size()};
+}
+
+template <class ELFT>
+typename ELFCreator<ELFT>::Symbol ELFCreator<ELFT>::addSymbol(StringRef Name) {
+  std::size_t NameOff = StrTabBuilder.add(Name);
+  auto Sym = new (Alloc) Elf_Sym{};
+  Sym->st_name = NameOff;
+  StaticSymbols.push_back(Sym);
+  return {Sym, StaticSymbols.size()};
+}
+
+template <class ELFT> std::size_t ELFCreator<ELFT>::layout() {
+  SecHdrStrTabBuilder.finalizeInOrder();
+  ShStrTab->sh_size = SecHdrStrTabBuilder.getSize();
+
+  StrTabBuilder.finalizeInOrder();
+  StrTab->sh_size = StrTabBuilder.getSize();
+
+  SymTab->sh_size = (StaticSymbols.size() + 1) * sizeof(Elf_Sym);
+
+  uintX_t Offset = sizeof(Elf_Ehdr);
+  for (Elf_Shdr *Sec : Sections) {
+    Offset = alignTo(Offset, Sec->sh_addralign);
+    Sec->sh_offset = Offset;
+    Offset += Sec->sh_size;
+  }
+
+  Offset = alignTo(Offset, sizeof(uintX_t));
+  Header.e_shoff = Offset;
+  Offset += (Sections.size() + 1) * sizeof(Elf_Shdr);
+  Header.e_shnum = Sections.size() + 1;
+
+  return Offset;
+}
+
+template <class ELFT> void ELFCreator<ELFT>::write(char *Out) {
+  std::memcpy(Out, &Header, sizeof(Elf_Ehdr));
+  std::copy(SecHdrStrTabBuilder.data().begin(),
+            SecHdrStrTabBuilder.data().end(), Out + ShStrTab->sh_offset);
+  std::copy(StrTabBuilder.data().begin(), StrTabBuilder.data().end(),
+            Out + StrTab->sh_offset);
+
+  Elf_Sym *Sym = reinterpret_cast<Elf_Sym *>(Out + SymTab->sh_offset);
+  // Skip null.
+  ++Sym;
+  for (Elf_Sym *S : StaticSymbols)
+    *Sym++ = *S;
+
+  Elf_Shdr *Shdr = reinterpret_cast<Elf_Shdr *>(Out + Header.e_shoff);
+  // Skip null.
+  ++Shdr;
+  for (Elf_Shdr *S : Sections)
+    *Shdr++ = *S;
+}
+
+template class elf::ELFCreator<ELF32LE>;
+template class elf::ELFCreator<ELF32BE>;
+template class elf::ELFCreator<ELF64LE>;
+template class elf::ELFCreator<ELF64BE>;

ELF/InputFiles.h

     LazyObjectKind,
     ArchiveKind,
     BitcodeKind,
+    BinaryKind,
   };
 
   Kind kind() const { return FileKind; }

   bool isNeeded() const { return !AsNeeded || IsUsed; }
 };
 
+class BinaryFile : public InputFile {
+public:
+  explicit BinaryFile(MemoryBufferRef M) : InputFile(BinaryKind, M) {}
+
+  static bool classof(const InputFile *F) { return F->kind() == BinaryKind; }
+
+  template <class ELFT> std::unique_ptr<InputFile> createELF();
+
+private:
+  std::unique_ptr<char[]> ELFData;
+};
+
 std::unique_ptr<InputFile> createObjectFile(MemoryBufferRef MB,
                                             StringRef ArchiveName = "");
 std::unique_ptr<InputFile> createSharedFile(MemoryBufferRef MB);

ELF/InputFiles.cpp

 #include "Error.h"
 #include "InputSection.h"
 #include "LinkerScript.h"
+#include "ELFCreator.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "llvm/ADT/STLExtras.h"

 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/MC/StringTableBuilder.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;

   return Obj;
 }
 
+template <class ELFT> std::unique_ptr<InputFile> BinaryFile::createELF() {
+  ELFCreator<ELFT> ELF(ET_REL, Config->EMachine);
+  auto DataSec = ELF.addSection(".data");
+  DataSec.Header->sh_flags = SHF_ALLOC;
+  DataSec.Header->sh_size = MB.getBufferSize();
+  DataSec.Header->sh_type = SHT_PROGBITS;
+  DataSec.Header->sh_addralign = 8;
+
+  std::string Filepath = MB.getBufferIdentifier();
+  std::transform(Filepath.begin(), Filepath.end(), Filepath.begin(),
+                 [](char C) { return isalnum(C) ? C : '_'; });
+  std::string StartSym = "_binary_" + Filepath + "_start";
+  std::string EndSym = "_binary_" + Filepath + "_end";
+  std::string SizeSym = "_binary_" + Filepath + "_size";
+
+  auto SSym = ELF.addSymbol(StartSym);
+  SSym.Sym->setBindingAndType(STB_GLOBAL, STT_OBJECT);
+  SSym.Sym->st_shndx = DataSec.Index;
+
+  auto ESym = ELF.addSymbol(EndSym);
+  ESym.Sym->setBindingAndType(STB_GLOBAL, STT_OBJECT);
+  ESym.Sym->st_shndx = DataSec.Index;
+  ESym.Sym->st_value = MB.getBufferSize();
+
+  auto SZSym = ELF.addSymbol(SizeSym);
+  SZSym.Sym->setBindingAndType(STB_GLOBAL, STT_OBJECT);
+  SZSym.Sym->st_shndx = SHN_ABS;
+  SZSym.Sym->st_value = MB.getBufferSize();
+
+  std::size_t Size = ELF.layout();
+  char *Buff = new char[Size]{};
+  ELFData.reset(Buff);
+
+  ELF.write(Buff);
+
+  // .data
+  std::copy(MB.getBufferStart(), MB.getBufferEnd(),
+            Buff + DataSec.Header->sh_offset);
+
+  return createELFFile<ObjectFile>(
+      MemoryBufferRef(StringRef(Buff, Size), MB.getBufferIdentifier()));
+}
+
 static bool isBitcode(MemoryBufferRef MB) {
   using namespace sys::fs;
   return identify_magic(MB.getBuffer()) == file_magic::bitcode;

 template class elf::SharedFile<ELF32BE>;
 template class elf::SharedFile<ELF64LE>;
 template class elf::SharedFile<ELF64BE>;
+
+template std::unique_ptr<InputFile> BinaryFile::createELF<ELF32LE>();
+template std::unique_ptr<InputFile> BinaryFile::createELF<ELF32BE>();
+template std::unique_ptr<InputFile> BinaryFile::createELF<ELF64LE>();
+template std::unique_ptr<InputFile> BinaryFile::createELF<ELF64BE>();

ELF/Options.td

 def fini: S<"fini">, MetaVarName<"<symbol>">,
   HelpText<"Specify a finalizer function">;
 
+def format: J<"format=">, MetaVarName<"<input-format>">,
+  HelpText<"Change the input format of the inputs following this option">;
+
 def gc_sections: F<"gc-sections">,
   HelpText<"Enable garbage collection of unused sections">;
 

 def alias_export_dynamic_symbol: J<"export-dynamic-symbol=">,
   Alias<export_dynamic_symbol>;
 def alias_fini_fini: J<"fini=">, Alias<fini>;
+def alias_format_b: S<"b">, Alias<format>;
 def alias_hash_style_hash_style: J<"hash-style=">, Alias<hash_style>;
 def alias_init_init: J<"init=">, Alias<init>;
 def alias_l__library: J<"library=">, Alias<l>;

ELF/SymbolTable.cpp

   if (!isCompatible<ELFT>(FileP))
     return;
 
+  // Binary file
+  if (auto *F = dyn_cast<BinaryFile>(FileP)) {
+    addFile(F->createELF<ELFT>());
+    return;
+  }
+
   // .a file
   if (auto *F = dyn_cast<ArchiveFile>(FileP)) {
     ArchiveFiles.emplace_back(cast<ArchiveFile>(File.release()));

test/elf/format-binary.test

+# REQUIRES: x86
+
+# RUN: echo -n "Fluffle Puff" > %t.binary
+# RUN: ld.lld -m elf_x86_64 -r -b binary %t.binary -o %t.out
+# RUN: llvm-readobj %t.out -sections -section-data -symbols | FileCheck %s
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld %t.o -b binary %t.binary -b default %t.o -shared -o %t.out
+
+# CHECK:          Name: .data
+# CHECK-NEXT:     Type: SHT_PROGBITS
+# CHECK-NEXT:     Flags [
+# CHECK-NEXT:       SHF_ALLOC
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Address: 0x0
+# CHECK-NEXT:     Offset:
+# CHECK-NEXT:     Size: 12
+# CHECK-NEXT:     Link: 0
+# CHECK-NEXT:     Info: 0
+# CHECK-NEXT:     AddressAlignment:
+# CHECK-NEXT:     EntrySize: 0
+# CHECK-NEXT:     SectionData (
+# CHECK-NEXT:       0000: 466C7566 666C6520 50756666           |Fluffle Puff|
+# CHECK-NEXT:     )
+# CHECK-NEXT:   }
+
+# CHECK:          Name: _binary_{{[a-zA-Z0-9_]+}}test_elf_Output_format_binary_test_tmp_binary_start
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Object
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _binary_{{[a-zA-Z0-9_]+}}test_elf_Output_format_binary_test_tmp_binary_end
+# CHECK-NEXT:     Value: 0xC
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Object
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _binary_{{[a-zA-Z0-9_]+}}test_elf_Output_format_binary_test_tmp_binary_size
+# CHECK-NEXT:     Value: 0xC
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Object
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Absolute
+# CHECK-NEXT:   }