Initialize Elf header to Zero

[rdhindsa]

Summary

Initialize Elf header to Zero to ensure that bytes not assigned any value later on are initialized properly.

[mgrang]

lld/ELF/Writer.cpp

2111 ↗

(On Diff #140098)

Please make spacing uniform.

memset(Buf, 0, sizeof(Elf_Ehdr) + sizeof(Elf_Phdr));

Also could you please add a unit test?

[ruiu]

lld/ELF/Writer.cpp

2111 ↗

(On Diff #140098)

If -r option is given, we don't emit Phdr, so this is inaccurate. It's in practice not a problem because I can't think of an ELF file that is smaller than sizeof(Elf_Ehdr) + sizeof(Elf_Phdr), but being accurate helps readers understand the code.

So it is better to just set the first sizeof(Elf_Ehdr).

[pcc]

I thought this was a memory-mapped file. Aren't its contents initialized to zero by the operating system?

[ruiu]

LGTM

lld/test/ELF/elf-header-initialized.s

1 ↗

(On Diff #140150)

I'd name this file just "elf-header.s".

4 ↗

(On Diff #140150)

Please remove '-r' as you are not using it.

7 ↗

(On Diff #140150)

Ditto

[pcc]

In D44986#1050562, @pcc wrote:

I thought this was a memory-mapped file. Aren't its contents initialized to zero by the operating system?

I would like an answer to this.

[pcc]

I see, it is because we write trap instructions to executable segments before writing the header. Please add a comment explaining this.

[pcc]

LGTM

[MaskRay]

template <class ELFT> void Writer<ELFT>::writeTrapInstr() {
  if (Script->HasSectionsCommand)
    return;

  // Fill the last page.
  uint8_t *Buf = Buffer->getBufferStart();
  for (PhdrEntry *P : Phdrs)
    if (P->p_type == PT_LOAD && (P->p_flags & PF_X)) ///////////////////
      fillTrap(Buf + alignDown(P->p_offset + P->p_filesz, Target->PageSize),
               Buf + alignTo(P->p_offset + P->p_filesz, Target->PageSize));

How about if (P->p_type == PT_LOAD && P->p_flags & PF_X && P->LastSec != Out::ProgramHeaders)

Other write* functions don't initialize the buffer to zero

[rdhindsa]

For some cases, eg with -r option, Phdr is not emitted. Hence it is better to initialize ELF header to zero instead.

lld/trunk/ELF/Writer.cpp

static uint8_t getAbiVersion() {
   if (Config->EMachine == EM_MIPS && getELFType() == ET_EXEC &&
       (Config->EFlags & (EF_MIPS_PIC | EF_MIPS_CPIC)) == EF_MIPS_CPIC)
     return 1;
   return 0;
 }
 
 template <class ELFT> void Writer<ELFT>::writeHeader() {
   uint8_t *Buf = Buffer->getBufferStart();
+  // For executable segments, the trap instructions are written before writing
+  // the header. Setting Elf header bytes to zero ensures that any unused bytes
+  // in header are zero-cleared, instead of having trap instructions.
+  memset(Buf, 0, sizeof(Elf_Ehdr));
   memcpy(Buf, "\177ELF", 4);
 
   // Write the ELF header.
   auto *EHdr = reinterpret_cast<Elf_Ehdr *>(Buf);
   EHdr->e_ident[EI_CLASS] = Config->Is64 ? ELFCLASS64 : ELFCLASS32;
   EHdr->e_ident[EI_DATA] = Config->IsLE ? ELFDATA2LSB : ELFDATA2MSB;
   EHdr->e_ident[EI_VERSION] = EV_CURRENT;
   EHdr->e_ident[EI_OSABI] = Config->OSABI;

lld/trunk/test/ELF/elf-header.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t1
+# RUN: llvm-readobj -file-headers %t1 | FileCheck %s
+
+# RUN: ld.lld %t.o -no-rosegment -o %t2
+# RUN: llvm-readobj -file-headers %t2 | FileCheck %s
+
+# CHECK:      ElfHeader {
+# CHECK-NEXT:   Ident {
+# CHECK-NEXT:     Magic: (7F 45 4C 46)
+# CHECK-NEXT:     Class: 64-bit (0x2)
+# CHECK-NEXT:     DataEncoding: LittleEndian (0x1)
+# CHECK-NEXT:     FileVersion: 1
+# CHECK-NEXT:     OS/ABI: SystemV (0x0)
+# CHECK-NEXT:     ABIVersion: 0
+# CHECK-NEXT:     Unused: (00 00 00 00 00 00 00)
+# CHECK-NEXT:   }