[ELF] - Implemented --oformat binary option.

[grimar]

Summary

-oformat output-format
`-oformat' option can be used to specify the binary format for the output object file.

Patch implements binary format output type.

[emaste]

For reference https://reviews.freebsd.org/D7409 is the proposed FreeBSD change to switch to a linker script for the boot components (and make use of --oformat binary).

[grimar]

In D23769#522445, @emaste wrote:

For reference https://reviews.freebsd.org/D7409 is the proposed FreeBSD change to switch to a linker script for the boot components (and make use of --oformat binary).

As I mentioned earlier in llvm-mails, gold implementation is differs with ld for --oformat binary.

I am looking on sys/boot/i386/boot.ldscript at the page you posted and see the next lines:

SECTIONS {
  . = 0x08048000 + SIZEOF_HEADERS;

That means that gold will produce output > 128mb while gnu ld will do the same layout as this patch do and output will depend only on size of sections.

I just re-checked this using stub files:
test.s:

.text
.globl _start
_start:

test.script:

SECTIONS { 
 . = 0x08048000 + SIZEOF_HEADERS;
 .text  : { *(.text*) }
}

llvm-mc -filetype=obj -triple=x86_64-pc-linux test.s -o test.o
gold --script test.script --oformat binary test.o -o outgold
Size of outgold is 134.5 MB for me, size of GNU ld output is 408 bytes.

So I think that means this patch do what is expected.

[ruiu]

ELF/Config.h

122 ↗

(On Diff #68857)

This can be bool OFormatBinary = false.

ELF/Driver.cpp

337 ↗

(On Diff #68857)

isOutputFormatBinary

338 ↗

(On Diff #68857)

This is technically not correct. If you pass --oformat "", it ignores the error. Please use Args.getLastArg(OPT_oformat) explicitly.

ELF/Writer.cpp

267–268 ↗

(On Diff #68857)

Do you have to not call this function? I don't like to sprinkle OFormatBinary check everywhere. If it works with this function call, remove this check.

1112 ↗

(On Diff #68857)

This seems to set only one offset, so setOffset?

[grimar]

ELF/Writer.cpp

267–268 ↗

(On Diff #68857)

I have to call it because it sets size for Out<ELFT>::ProgramHeaders:

Out<ELFT>::ProgramHeaders->setSize(sizeof(Elf_Phdr) * Phdrs.size());

What affects on:

template <class ELFT> void Writer<ELFT>::assignAddresses() {
  uintX_t VA = Config->ImageBase + Out<ELFT>::ElfHeader->getSize() +
               Out<ELFT>::ProgramHeaders->getSize();

I think this minimal change is better then add code to avoid using these 2 special sections at all ?

[ruiu]

ELF/Writer.cpp

1071–1072 ↗

(On Diff #68857)

It seems to me that

uintX_t VA = Config->ImageBase;
if (!OFormatBinary)
  VA += Out<ELFT>::ElfHeader->getSize() +
             Out<ELFT>::ProgramHeaders->getSize();

is easier to understand.

[grimar]

ELF/Writer.cpp

1071–1072 ↗

(On Diff #68857)

It is not only that place unfortunately. These sections are added to output sections as well and that brings more changes. I can update this patch to avoid

if (Config->OFormat == OutputFormat::Elf)
      fixHeaders();

just to show the amount of changes required.

[grimar]

• Addressed review comments.
• Improved testcase.

ELF/Config.h

122 ↗

(On Diff #68857)

Done.

ELF/Driver.cpp

338 ↗

(On Diff #68857)

Fixed.

ELF/Writer.cpp

1071–1072 ↗

(On Diff #68857)

Hmm, I was mistaken, this change should be enough.
It is definetely possible to live without disabling fixHeaders() and changes at Writer.cpp(131).
Troubles I had were because of another changes I tried during development. And I was incorrect when was saying "These sections are added to output sections as well", I was thinking about PHDRS at that moment, but we do not write them in this patch, so should not care.

1112 ↗

(On Diff #68857)

Done.

[ruiu]

LGTM. Thank you for doing this!

ELF/Config.h

95 ↗

(On Diff #69107)

Remove = false.

lld/trunk/ELF/Config.h

struct Configuration {
   bool ExportDynamic;
   bool FatalWarnings;
   bool GcSections;
   bool GnuHash = false;
   bool ICF;
   bool Mips64EL = false;
   bool NoGnuUnique;
   bool NoUndefinedVersion;
+  bool OFormatBinary;
   bool Pic;
   bool Pie;
   bool PrintGcSections;
   bool Rela;
   bool Relocatable;
   bool SaveTemps;
   bool Shared;
   bool Static = false;

lld/trunk/ELF/Driver.cpp

if (S == "ignore-all" || S == "ignore-in-object-files")
       return UnresolvedPolicy::Ignore;
     if (S == "ignore-in-shared-libs" || S == "report-all")
       return UnresolvedPolicy::Error;
     error("unknown --unresolved-symbols value: " + S);
   }
   return UnresolvedPolicy::Error;
 }
 
+static bool isOutputFormatBinary(opt::InputArgList &Args) {
+  if (auto *Arg = Args.getLastArg(OPT_oformat)) {
+    StringRef S = Arg->getValue();
+    if (S == "binary")
+      return true;
+    error("unknown --oformat value: " + S);
+  }
+  return false;
+}
+
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   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 (auto *Arg = Args.getLastArg(OPT_build_id_eq)) {
     } else if (S.startswith("0x")) {
       Config->BuildId = BuildIdKind::Hexstring;
       Config->BuildIdVector = parseHex(S.substr(2));
     } else {
       error("unknown --build-id style: " + S);
     }
   }
 
+  Config->OFormatBinary = isOutputFormatBinary(Args);
+
   for (auto *Arg : Args.filtered(OPT_undefined))
     Config->Undefined.push_back(Arg->getValue());
 
   Config->UnresolvedSymbols = getUnresolvedSymbolOption(Args);
 
   if (auto *Arg = Args.getLastArg(OPT_dynamic_list))
     if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
       parseDynamicList(*Buffer);

lld/trunk/ELF/Options.td

def no_undefined: F<"no-undefined">,
   HelpText<"Report unresolved symbols even if the linker is creating a shared library">;
 
 def no_undefined_version: F<"no-undefined-version">,
   HelpText<"Report version scripts that refer undefined symbols">;
 
 def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 
+def oformat: Separate<["--"], "oformat">, MetaVarName<"<format>">,
+  HelpText<"Specify the binary format for the output object file">;
+  
 def pie: F<"pie">, HelpText<"Create a position independent executable">;
 
 def print_gc_sections: F<"print-gc-sections">,
   HelpText<"List removed unused sections">;
 
 def reproduce: S<"reproduce">,
   HelpText<"Dump linker invocation and input files for debugging">;
 

lld/trunk/ELF/Writer.cpp

void forEachRelSec(
           Fn);
   void finalizeSections();
   void addPredefinedSections();
   bool needsGot();
 
   std::vector<Phdr> createPhdrs();
   void assignAddresses();
   void assignFileOffsets();
+  void assignFileOffsetsBinary();
   void setPhdrs();
   void fixHeaders();
   void fixSectionAlignments();
   void fixAbsoluteSymbols();
   void openFile();
   void writeHeader();
   void writeSections();
+  void writeSectionsBinary();
   void writeBuildId();
 
   std::unique_ptr<FileOutputBuffer> Buffer;
 
   BumpPtrAllocator Alloc;
   std::vector<OutputSectionBase<ELFT> *> OutputSections;
   OutputSectionFactory<ELFT> Factory;
 
Phdrs = Script<ELFT>::X->hasPhdrsCommands() ? Script<ELFT>::X->createPhdrs()
                                                 : createPhdrs();
     fixHeaders();
     if (ScriptConfig->HasContents) {
       Script<ELFT>::X->assignAddresses();
     } else {
       fixSectionAlignments();
       assignAddresses();
     }
+
+    if (!Config->OFormatBinary)
       assignFileOffsets();
+    else
+      assignFileOffsetsBinary();
+
     setPhdrs();
     fixAbsoluteSymbols();
   }
 
   openFile();
   if (HasError)
     return;
+  if (!Config->OFormatBinary) {
     writeHeader();
     writeSections();
+  } else {
+    writeSectionsBinary();
+  }
   writeBuildId();
   if (HasError)
     return;
   if (auto EC = Buffer->commit())
     error(EC, "failed to write to the output file");
 }
 
 template <class ELFT> static void reportUndefined(SymbolBody *Sym) {
template <class ELFT> void Writer<ELFT>::fixHeaders() {
   Out<ELFT>::ElfHeader->setVA(BaseVA);
   uintX_t Off = Out<ELFT>::ElfHeader->getSize();
   Out<ELFT>::ProgramHeaders->setVA(Off + BaseVA);
   Out<ELFT>::ProgramHeaders->setSize(sizeof(Elf_Phdr) * Phdrs.size());
 }
 
 // Assign VAs (addresses at run-time) to output sections.
 template <class ELFT> void Writer<ELFT>::assignAddresses() {
-  uintX_t VA = Config->ImageBase + Out<ELFT>::ElfHeader->getSize() +
-               Out<ELFT>::ProgramHeaders->getSize();
+  uintX_t VA = Config->ImageBase;
+  if (!Config->OFormatBinary)
+    VA +=
+        Out<ELFT>::ElfHeader->getSize() + Out<ELFT>::ProgramHeaders->getSize();
 
   uintX_t ThreadBssOffset = 0;
   for (OutputSectionBase<ELFT> *Sec : OutputSections) {
     uintX_t Alignment = Sec->getAlignment();
     if (Sec->PageAlign)
       Alignment = std::max<uintX_t>(Alignment, Target->PageSize);
 
     // We only assign VAs to allocated sections.
static uintX_t getFileAlignment(uintX_t Off, OutputSectionBase<ELFT> *Sec) {
 
   // Relocatable output does not have program headers
   // and does not need any other offset adjusting.
   if (Config->Relocatable || !(Sec->getFlags() & SHF_ALLOC))
     return Off;
   return alignTo(Off, Target->PageSize, Sec->getVA());
 }
 
-// Assign file offsets to output sections.
-template <class ELFT> void Writer<ELFT>::assignFileOffsets() {
-  uintX_t Off = 0;
-
-  auto Set = [&](OutputSectionBase<ELFT> *Sec) {
-    if (Sec->getType() == SHT_NOBITS) {
-      Sec->setFileOffset(Off);
-      return;
-    }
+template <class ELFT, class uintX_t>
+void setOffset(OutputSectionBase<ELFT> *Sec, uintX_t &Off) {
 
-    Off = getFileAlignment<ELFT>(Off, Sec);
-    Sec->setFileOffset(Off);
-    Off += Sec->getSize();
-  };
+}
 
-  Set(Out<ELFT>::ElfHeader);
-  Set(Out<ELFT>::ProgramHeaders);
+template <class ELFT> void Writer<ELFT>::assignFileOffsetsBinary() {
+  uintX_t Off = 0;
   for (OutputSectionBase<ELFT> *Sec : OutputSections)
-    Set(Sec);
+    if (Sec->getFlags() & SHF_ALLOC)
+      setOffset(Sec, Off);
+  FileSize = alignTo(Off, sizeof(uintX_t));
+}
+
+// Assign file offsets to output sections.
+template <class ELFT> void Writer<ELFT>::assignFileOffsets() {
+  uintX_t Off = 0;
+  setOffset(Out<ELFT>::ElfHeader, Off);
+  setOffset(Out<ELFT>::ProgramHeaders, Off);
+
+  for (OutputSectionBase<ELFT> *Sec : OutputSections)
+    setOffset(Sec, Off);
 
   SectionHeaderOff = alignTo(Off, sizeof(uintX_t));
   FileSize = SectionHeaderOff + (OutputSections.size() + 1) * sizeof(Elf_Shdr);
 }
 
 // Finalize the program headers. We call this function after we assign
 // file offsets and VAs to all sections.
 template <class ELFT> void Writer<ELFT>::setPhdrs() {
ErrorOr<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
       FileOutputBuffer::create(Config->OutputFile, FileSize,
                                FileOutputBuffer::F_executable);
   if (auto EC = BufferOrErr.getError())
     error(EC, "failed to open " + Config->OutputFile);
   else
     Buffer = std::move(*BufferOrErr);
 }
 
+template <class ELFT> void Writer<ELFT>::writeSectionsBinary() {
+  uint8_t *Buf = Buffer->getBufferStart();
+  for (OutputSectionBase<ELFT> *Sec : OutputSections)
+    if (Sec->getFlags() & SHF_ALLOC)
+      Sec->writeTo(Buf + Sec->getFileOff());
+}
+
 // Write section contents to a mmap'ed file.
 template <class ELFT> void Writer<ELFT>::writeSections() {
   uint8_t *Buf = Buffer->getBufferStart();
 
   // PPC64 needs to process relocations in the .opd section
   // before processing relocations in code-containing sections.
   Out<ELFT>::Opd = findSection(".opd");
   if (Out<ELFT>::Opd) {

lld/trunk/test/ELF/oformat-binary.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+
+# RUN: ld.lld -o %t.out %t --oformat binary
+# RUN: hexdump -C %t.out | FileCheck %s
+# CHECK: 00000000 90 11 22 00 00 00 00 00
+# CHECK-NOT: 00000010
+
+## Check case when linkerscript is used.
+# RUN: echo "SECTIONS { . = 0x1000; }" > %t.script
+# RUN: ld.lld -o %t2.out --script %t.script %t --oformat binary
+# RUN: hexdump -C %t2.out | FileCheck %s
+
+# RUN: not ld.lld -o %t3.out %t --oformat foo 2>&1 \
+# RUN:   | FileCheck %s --check-prefix ERR
+# ERR: unknown --oformat value: foo
+
+.text
+.align 4
+.globl _start
+_start:
+ nop
+
+.section        .mysec.1,"ax"
+.byte   0x11
+
+.section        .mysec.2,"ax"
+.byte   0x22