[ELF] Change isSectionData to exclude SHF_EXECINSTR

[MaskRay]

Summary

This affects what sections are displayed as "DATA" in llvm-objdump.
The other user llvm-size is unaffected.

Before, a "TEXT" section is also "DATA", which seems weird.
The sh_flags condition matches that of bfd's SEC_DATA but the sh_type
condition uses (== SHF_PROGBITS) instead of bfd's (!= SHT_NOBITS).
bfd's SEC_DATA is not appealing as so many sections will be shown as DATA.

[MaskRay]

Update isSectionBSS as well (no impact)

[jyknight]

I guess it's better...I mean, seems reasonable, I guess.

But I'd really *much* rather see the *actual* type information from the object file, not invent arbitrary classifications. E.g., how about we just print PROGBITS, NOBITS, etc.

And it'd be nice to see all the other section attributes too (flags/info/link/etc), for that matter...

[MaskRay]

I agree with jyknight on this.. "TEXT" "DATA" "BSS" add an unnecessary abstraction layer on top of SHT_PROGBITS SHF_EXECINSTR ... If we could start over, we should remove the abstraction and change the display.

Thankfully most of this usage is superseded by llvm-readelf . Chime in freebsd hackers as there is recently discussion on moving away from GNU binutils https://lists.freebsd.org/pipermail/freebsd-toolchain/2018-June/003809.html

include/llvm/Object/ELFObjectFile.h

 template <class ELFT>
 bool ELFObjectFile<ELFT>::isSectionText(DataRefImpl Sec) const {
   return getSection(Sec)->sh_flags & ELF::SHF_EXECINSTR;
 }
 
 template <class ELFT>
 bool ELFObjectFile<ELFT>::isSectionData(DataRefImpl Sec) const {
   const Elf_Shdr *EShdr = getSection(Sec);
-  return EShdr->sh_flags & (ELF::SHF_ALLOC | ELF::SHF_WRITE) &&
-         EShdr->sh_type == ELF::SHT_PROGBITS;
+  return EShdr->sh_type == ELF::SHT_PROGBITS &&
+         EShdr->sh_flags & ELF::SHF_ALLOC &&
+         !(EShdr->sh_flags & ELF::SHF_EXECINSTR);
 }
 
 template <class ELFT>
 bool ELFObjectFile<ELFT>::isSectionBSS(DataRefImpl Sec) const {
   const Elf_Shdr *EShdr = getSection(Sec);
-  return EShdr->sh_flags & (ELF::SHF_ALLOC | ELF::SHF_WRITE) &&
-         EShdr->sh_type == ELF::SHT_NOBITS;
+  return EShdr->sh_type == ELF::SHT_NOBITS &&
+         EShdr->sh_flags & ELF::SHF_ALLOC;
 }
 
 template <class ELFT>
 std::vector<SectionRef>
 ELFObjectFile<ELFT>::dynamic_relocation_sections() const {
   std::vector<SectionRef> Res;
   std::vector<uintptr_t> Offsets;
 

test/Object/objdump-sectionheaders.test

 ; RUN: llvm-objdump -h %p/Inputs/trivial-object-test.elf-x86-64 \
 ; RUN:              | FileCheck %s
 
 ; To verify this, use readelf -S, not objdump -h. Binutils objdump filters the
 ; results in a way that we don't emulate.
 
 ; CHECK: Sections:
 ; CHECK: Idx Name          Size      Address          Type
 ; CHECK:   0               00000000 0000000000000000
-; CHECK:   1 .text         00000026 0000000000000000 TEXT DATA
+; CHECK:   1 .text         00000026 0000000000000000 TEXT
 ; CHECK:   2 .rodata.str1.1 0000000d 0000000000000026 DATA
 ; CHECK:   3 .note.GNU-stack 00000000 0000000000000033
 ; CHECK:   4 .rela.text    00000048 0000000000000038
 ; CHECK:   5 .symtab       000000c0 0000000000000080
 ; CHECK:   6 .strtab       00000033 0000000000000140
 ; CHECK:   7 .shstrtab     0000004b 0000000000000173