[objcopy] make objcopy follow program header standards

[amontanez]

Summary

Objects with unused program headers copied by objcopy would always have nonzero values for program header offset and program header entry size. While technically valid, this atypical behavior triggers warnings in some tools. This change sets the two fields to zero when the program header is unused, better fitting the general expectations for unused program header data.

Section headers behaved somewhat similarly (though only with the entry size), and are fixed in this revision as well.

[jakehehrlich]

A couple nits but this LGTM.

llvm/test/tools/llvm-objcopy/relocatable-phdr.test

5

You'll want to use llvm-readobj instead of llvm-readelf. It produces a more machine readable format.

llvm/tools/llvm-objcopy/Object.cpp

1053

I'd use phnum here instead of recalling size.

[rupprecht]

Thanks for fixing this!

[amontanez]

Now using llvm-readobj instead of llvm-readelf for test, updated patch to use Ehdr.e_phnum instead of calling size() repeatedly.

[amontanez]

Done, good points.

[jakehehrlich]

When you write your diff use git diff -U<some big number> origin/master so that the diff is relative to what's upstream not your last change locally. I think git show might be understood by fabricator as well but I'm not 100% sure.

llvm/test/tools/llvm-objcopy/relocatable-phdr.test

16

nit: add a space here

[jakehehrlich]

Oh also LGTM

[mcgrathr]

llvm/tools/llvm-objcopy/Object.cpp

1057

While you're at it, don't set shentsize when there are no sections.

[jakehehrlich]

llvm/tools/llvm-objcopy/Object.cpp

1057

You'll want to use !WriteSectionHeaders || size(Obj.sections()) == 0 as the condition for that BTW. WriteSectionHeaders is normally true but is false when --strip-sections is used. WriteSectionHeaders isn't set to false when size(Obj.sections()) == 0 is true

Thinking about it now the if (WriteSectionHeaders) below should probably be WriteSectionHeaders && size(Obj.sections()) != 0 and then you can just set e_shentsize inside each branch of that if statement. That way if a sectionless object is input to llvm-objcopy it will output it with all those fields set to zero even though --strip-sections wasn't used.

[jakehehrlich]

I'll LGTM after Roland's fixes are done since I agree, that would be nice to also fix.

[amontanez]

Fixes affected test cases, now includes fix for similar behavior from section headers.

[amontanez]

Done! I ran all the tests for llvm-objcopy and updated everything that was failing from these changes.

[jakehehrlich]

LGTM.

llvm/test/tools/llvm-objcopy/relocatable-phdr.test

 # This test ensures an object without a program header will retain zero values
 # for offset and header size when copied with llvm-objcopy.
 # RUN: yaml2obj %s > %t
 # RUN: llvm-objcopy %t %t2
-# RUN: llvm-readelf -file-headers %t2 | FileCheck %s
+# RUN: llvm-readobj -file-headers %t2 | FileCheck %s
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_EXEC
   Machine:         EM_X86_64
 
-# CHECK:     Start of program headers:          0 (bytes into file)
-# CHECK:     Size of program headers:           0 (bytes)
-# CHECK-NEXT:Number of program headers:         0
+# CHECK:     ProgramHeaderOffset: 0x0
+# CHECK:     ProgramHeaderEntrySize: 0
+# CHECK-NEXT:ProgramHeaderCount: 0

llvm/tools/llvm-objcopy/Object.cpp

template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
   Ehdr.e_ident[EI_OSABI] = ELFOSABI_NONE;
   Ehdr.e_ident[EI_ABIVERSION] = 0;
 
   Ehdr.e_type = Obj.Type;
   Ehdr.e_machine = Obj.Machine;
   Ehdr.e_version = Obj.Version;
   Ehdr.e_entry = Obj.Entry;
   Ehdr.e_phnum = size(Obj.segments());
-  Ehdr.e_phoff = (size(Obj.segments()) != 0) ? Obj.ProgramHdrSegment.Offset : 0;
-  Ehdr.e_phentsize = (size(Obj.segments()) != 0) ? sizeof(Elf_Phdr) : 0;
+  Ehdr.e_phoff = (Ehdr.e_phnum != 0) ? Obj.ProgramHdrSegment.Offset : 0;
+  Ehdr.e_phentsize = (Ehdr.e_phnum != 0) ? sizeof(Elf_Phdr) : 0;
   Ehdr.e_flags = Obj.Flags;
   Ehdr.e_ehsize = sizeof(Elf_Ehdr);
   Ehdr.e_shentsize = sizeof(Elf_Shdr);
   if (WriteSectionHeaders) {
     Ehdr.e_shoff = Obj.SHOffset;
     // """
     // If the number of sections is greater than or equal to
     // SHN_LORESERVE (0xff00), this member has the value zero and the actual
     // number of section header table entries is contained in the sh_size field
     // of the section header at index 0.
     // """