[llvm-readelf] - Change letters used for SHF_ARM_PURECODE and SHF_X86_64_LARGE flags.

[grimar]

Summary

GNU uses l for SHF_X86_64_LARGE and y for SHF_ARM_PURECODE.
Lets follow.

To do this I had to refactor and refine how we print the help flags description.
It was too generic and inconsistent with GNU readelf.

Depends on https://reviews.llvm.org/D71462

[grimar]

I've found that we also seems need to update "Key to Flags:" printed

For example. when SHF_ARM_PURECODE is used, GNU readelf prints:

Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  y (purecode), p (processor specific)

When SHF_X86_64_LARGE is used, it prints:

Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  l (large), p (processor specific)

I am doing to update the patch with this change.

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

1505

Str += 'l'

1509

SHF_ARM_PURECODE

[grimar]

• Addressed comments.
• Fixed the flags description printed.

llvm/tools/llvm-readobj/ELFDumper.cpp

1509

I've removed these lines. They are not needed actually because of Flags &= ~Flag.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/gnu-sections.test

1 ↗

(On Diff #234034)

section and flag descriptions

4 ↗

(On Diff #234034)

I'd probably delete the phrase "help text" here as it could be confused with the output of --help, and is unnecessary ("key" already implies its a descriptive piece of help text).

I'd also rephrase slightly:
"to show how the default flag key is printed."

7 ↗

(On Diff #234034)

--check-prefix here and below.

61 ↗

(On Diff #234034)

a EM_X86_64 -> an EM_X86_64
for the SHF_X86_64_LARGE

62 ↗

(On Diff #234034)

Check we mention it in the flag key.

127 ↗

(On Diff #234034)

a EM_ARM -> an EM_ARM
for the SHF_ARM_PURECODE

128 ↗

(On Diff #234034)

Same comment as above.

llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test

73

Why does it look strange? (Also "Despite the fact...")

[grimar]

llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test

73

Because we print o, p and x last, so one might think that the order is UPPER CASE letters and lower case after them.
But lE violates this.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test

73

Okay, I'd change this statement to simply say something like:

"GNU prints 'l' before 'E', despite otherwise printing upper-case letters first".

[grimar]

• Addressed review comments.

[jhenderson]

LGTM, except for one comment.

llvm/test/tools/llvm-readobj/ELF/gnu-sections.test

1 ↗

(On Diff #234057)

If this should be interpreted as "section descriptions and flag descriptions", then sections -> section.

llvm/test/MC/ARM/elf-execute-only-section.ll

 ; RUN: llc < %s -mtriple=thumbv8m.base-eabi -mattr=+execute-only -filetype=obj %s -o - | \
 ; RUN: llvm-readelf -S | FileCheck %s
 ; RUN: llc < %s -mtriple=thumbv8m.main-eabi -mattr=+execute-only -filetype=obj %s -o - | \
 ; RUN: llvm-readelf -S | FileCheck %s
 ; RUN: llc < %s -mtriple=thumbv7m-eabi -mattr=+execute-only -filetype=obj %s -o - | \
 ; RUN: llvm-readelf -S | FileCheck %s
 
-; CHECK-NOT: {{.text[ ]+PROGBITS[ ]+[0-9]+ [0-9]+ [0-9]+ [0-9]+ AX[^p]}}
-; CHECK: {{.text[ ]+PROGBITS[ ]+[0-9]+ [0-9]+ [0-9]+ [0-9]+ AXp}}
+; CHECK-NOT: {{.text[ ]+PROGBITS[ ]+[0-9]+ [0-9]+ [0-9]+ [0-9]+ AX[^y]}}
+; CHECK: {{.text[ ]+PROGBITS[ ]+[0-9]+ [0-9]+ [0-9]+ [0-9]+ AXy}}
 define void @test_func() {
 entry:
   ret void
 }

llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test

- Name:  .mips
     Type:  SHT_PROGBITS
     Flags: [ SHF_MIPS_NODUPES, SHF_MIPS_NAMES, SHF_MIPS_LOCAL, SHF_MIPS_NOSTRIP,
               SHF_MIPS_GPREL, SHF_MIPS_MERGE, SHF_MIPS_ADDR, SHF_MIPS_STRING ]
 
 # RUN: yaml2obj --docnum 3 %s -o %t-x86_64.o
 # RUN: llvm-readobj -S %t-x86_64.o | FileCheck --check-prefix=X86_64-LLVM %s
 # RUN: llvm-readelf -S %t-x86_64.o | FileCheck --check-prefix=X86_64-GNU %s
 
-# X86_64-LLVM:      Flags [ (0x10000000)
+# X86_64-LLVM:      Name: .x86_64
+# X86_64-LLVM-NEXT: Type: SHT_PROGBITS
+# X86_64-LLVM-NEXT: Flags [ (0x10000000)
 # X86_64-LLVM-NEXT:   SHF_X86_64_LARGE (0x10000000)
 # X86_64-LLVM-NEXT: ]
 
-# X86_64-GNU: [Nr] Name    Type     Address          Off    Size   ES Flg Lk Inf Al
-# X86_64-GNU: [ 1] .x86_64 PROGBITS 0000000000000000 000040 000000 00   p 0   0  0
+# X86_64-LLVM:      Name: .with.exclude
+# X86_64-LLVM-NEXT: Type: SHT_PROGBITS
+# X86_64-LLVM-NEXT: Flags [ (0x90000000)
+# X86_64-LLVM-NEXT:   SHF_EXCLUDE (0x80000000)
+# X86_64-LLVM-NEXT:   SHF_X86_64_LARGE (0x10000000)
+# X86_64-LLVM-NEXT: ]
+
+## Despite of the fact that "lE" looks a bit strange, this matches the GNU order.
+# X86_64-GNU: [Nr] Name          Type     Address          Off    Size   ES Flg Lk Inf Al
+# X86_64-GNU: [ 1] .x86_64       PROGBITS 0000000000000000 000040 000000 00   l 0   0  0
+# X86_64-GNU: [ 2] .with.exclude PROGBITS 0000000000000000 000040 000000 00  lE 0   0  0
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name:  .x86_64
     Type:  SHT_PROGBITS
     Flags: [ SHF_X86_64_LARGE ]
+  - Name:  .with.exclude
+    Type:  SHT_PROGBITS
+    Flags: [ SHF_X86_64_LARGE, SHF_EXCLUDE ]
 
 # RUN: yaml2obj --docnum 4 %s -o %t-arm.o
 # RUN: llvm-readobj -S %t-arm.o | FileCheck --check-prefix=ARM-LLVM %s
 # RUN: llvm-readelf -S %t-arm.o | FileCheck --check-prefix=ARM-GNU %s
 
 # ARM-LLVM:      Flags [ (0x20000000)
 # ARM-LLVM-NEXT:   SHF_ARM_PURECODE (0x20000000)
 # ARM-LLVM-NEXT: ]
 
 # ARM-GNU: [Nr] Name Type     Address          Off    Size   ES Flg Lk Inf Al
-# ARM-GNU: [ 1] foo  PROGBITS 0000000000000000 000040 000000 00   p 0   0  0
+# ARM-GNU: [ 1] foo  PROGBITS 0000000000000000 000040 000000 00   y 0   0  0
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_ARM
 Sections:

llvm/tools/llvm-readobj/ELFDumper.cpp

static const EnumEntry<unsigned> ElfMipsSectionFlags[] = {
   LLVM_READOBJ_ENUM_ENT(ELF, SHF_MIPS_ADDR   ),
   LLVM_READOBJ_ENUM_ENT(ELF, SHF_MIPS_STRING )
 };
 
 static const EnumEntry<unsigned> ElfX86_64SectionFlags[] = {
   LLVM_READOBJ_ENUM_ENT(ELF, SHF_X86_64_LARGE)
 };
 
-static std::string getGNUFlags(uint64_t Flags) {
+static std::string getGNUFlags(unsigned EMachine, uint64_t Flags) {
   // Here we are trying to build the flags string in the same way as GNU does.
   // It is not that straightforward. Imagine we have sh_flags == 0x90000000.
   // SHF_EXCLUDE ("E") has a value of 0x80000000 and SHF_MASKPROC is 0xf0000000.
   // GNU readelf will not print "E" or "Ep" in this case, but will print just
   // "p". It only will print "E" when no other processor flag is set.
   std::string Str;
   bool HasUnknownFlag = false;
   bool HasOSFlag = false;
while (Flags) {
     if (I != std::end(ElfSectionFlags)) {
       Flags &= ~I->Value;
       Str += I->AltName;
       continue;
     }
 
     // If we did not find a matching regular flag, then we deal with a OS
     // specific flag, proccessor specific flag or an unknown flag.
-    if (Flag & ELF::SHF_MASKOS) {
+    if (EMachine == EM_X86_64 && Flag == SHF_X86_64_LARGE) {
+      Str += "l";
+      Flags &= ~ELF::SHF_X86_64_LARGE;
+    } else if (EMachine == EM_ARM && Flag == SHF_ARM_PURECODE) {
+      Str += "y";
+      Flags &= ~ELF::SHF_X86_64_LARGE;
+    } else if (Flag & ELF::SHF_MASKOS) {
       HasOSFlag = true;
       Flags &= ~ELF::SHF_MASKOS;
     } else if (Flag & ELF::SHF_MASKPROC) {
       HasProcFlag = true;
       Flags &= ~ELF::SHF_MASKPROC;
     } else {
       HasUnknownFlag = true;
     }
Fields[1].Str = unwrapOrError<StringRef>(
         ElfObj->getFileName(), Obj->getSectionName(&Sec, this->WarningHandler));
     Fields[2].Str =
         getSectionTypeString(Obj->getHeader()->e_machine, Sec.sh_type);
     Fields[3].Str =
         to_string(format_hex_no_prefix(Sec.sh_addr, ELFT::Is64Bits ? 16 : 8));
     Fields[4].Str = to_string(format_hex_no_prefix(Sec.sh_offset, 6));
     Fields[5].Str = to_string(format_hex_no_prefix(Sec.sh_size, 6));
     Fields[6].Str = to_string(format_hex_no_prefix(Sec.sh_entsize, 2));
-    Fields[7].Str = getGNUFlags(Sec.sh_flags);
+    Fields[7].Str = getGNUFlags(Obj->getHeader()->e_machine, Sec.sh_flags);
     Fields[8].Str = to_string(Sec.sh_link);
     Fields[9].Str = to_string(Sec.sh_info);
     Fields[10].Str = to_string(Sec.sh_addralign);
 
     OS.PadToColumn(Fields[0].Column);
     OS << "[" << right_justify(Fields[0].Str, 2) << "]";
     for (int i = 1; i < 7; i++)
       printField(Fields[i]);