HomePhabricator

[llvm-objcopy] Simplify SHT_NOBITS -> SHT_PROGBITS promotion

Authored by MaskRay on Apr 30 2019, 5:39 PM.

Description

[llvm-objcopy] Simplify SHT_NOBITS -> SHT_PROGBITS promotion

GNU objcopy uses bfd_elf_get_default_section_type to decide the candidate section type,
which roughly translates to our [a] (I assume SEC_COMMON implies SHF_ALLOC):

(!(Sec.Flags & ELF::SHF_ALLOC) || Flags & (SectionFlag::SecContents | SectionFlag::SecLoad)))

Then, it updates the section type in bfd/elf.c:elf_fake_sections if:

if (this_hdr->sh_type == SHT_NULL)
  this_hdr->sh_type = sh_type; // common case
else if (this_hdr->sh_type == SHT_NOBITS
         && sh_type == SHT_PROGBITS
         && (asect->flags & SEC_ALLOC) != 0)  // uncommon case
  ...
  this_hdr->sh_type = sh_type;

If the following condition is met the uncommon branch is executed:

  if (elf_section_type (osec) == SHT_NULL
      && (osec->flags == isec->flags
	  || (final_link
	      && ((osec->flags ^ isec->flags)
		  & ~(SEC_LINK_ONCE | SEC_LINK_DUPLICATES | SEC_RELOC)) == 0)))

I suggest we just ignore this clause and follow the common case
behavior, which is done in this patch. Rationales to do so:

If --set-section-flags is a no-op (osec->flags == isec->flags)
(corresponds to the "readonly" test in set-section-flags.test), GNU
objcopy will require (Sec.Flags & ELF::SHF_ALLOC). [a] is essentially:

Flags & (SectionFlag::SecContents | SectionFlag::SecLoad)

This special case is not really useful. Non-SHF_ALLOC SHT_NOBITS
sections do not make much sense and it doesn't matter if they are
SHT_NOBITS or SHT_PROGBITS.

For all other RUN lines in set-section-flags.test, the new behavior
matches GNU objcopy, i.e. this patch improves compatibility.

Differential Revision: https://reviews.llvm.org/D60189

llvm-svn: 359639

Details

Committed
MaskRayApr 30 2019, 5:39 PM
Differential Revision
D60189: [llvm-objcopy] Simplify SHT_NOBITS -> SHT_PROGBITS promotion
Parents
rGc0806e0d24ad: [libFuzzer] Fix failing test: sigint.test
Branches
Unknown
Tags
Unknown