[PATCH] D71462: [llvm-readelf][llvm-readobj] - Reimplement the logic of section flags dumping.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.





================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test:120
+# OS-PROC-LLVM-NEXT: Type: SHT_PROGBITS
+# OS-PROC-LLVM-NEXT: Flags [ (0xFE00000)
+# OS-PROC-LLVM-NEXT: ]
----------------
jhenderson wrote:
> What is this test case trying to show?
I wanted to show how we handle the value that is (`SHF_MASKOS` - 1).
I supposed it is reasonable, because we might have some logic (and we actually have it)
that works with `SHF_MASKOS` in the code. Hence I tried to check the bounds: a minimal possible value,
the mask itself and something that is almost large as `SHF_MASKOS`, but `!= SHF_MASKOS`.

Do you think it is excessive?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test:132
+# OS-PROC-LLVM-NEXT: Type: SHT_PROGBITS
+# OS-PROC-LLVM-NEXT: Flags [ (0xE0000000)
+# OS-PROC-LLVM-NEXT:   SHF_EXCLUDE (0x80000000)
----------------
jhenderson wrote:
> Same as above. What is this case trying to show?
The same, it is `SHF_MASKPROC` - 1.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test:151
+# OS-PROC-LLVM-NEXT: Type: SHT_PROGBITS
+# OS-PROC-LLVM-NEXT: Flags [ (0xEFE00000)
+# OS-PROC-LLVM-NEXT:   SHF_EXCLUDE (0x80000000)
----------------
jhenderson wrote:
> Same as above. What is this case actually trying to show?
It's just both `SHF_MASKPROC - 1` and `SHF_MASKOS - 1` together.
for consistency of `.both.flags*` and `.proc.flags*`/`.os.flags*` tests.

This might test we drop the bits that belong to the right mask probably.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-arch-flags.test:172
+# OS-PROC-GNU-NEXT:  [ 9] .both.flags.maxvalues PROGBITS 0000000000000000 000040 000000 00  op 0   0  0
+# OS-PROC-GNU-NEXT:  [10] .exclude              PROGBITS 0000000000000000 000040 000000 00   E 0   0  0
+
----------------
jhenderson wrote:
> What should GNU output do for a MIPS object with SHF_EXCLUDE set, since that value is also a used processor-specific one?
If we have:

```
--- !ELF
FileHeader:
  Class:   ELFCLASS32
  Data:    ELFDATA2LSB
  Type:    ET_REL
  Machine: EM_MIPS
Sections:
  - Name:  .mips
    Type:  SHT_PROGBITS
    Flags: [ SHF_MIPS_STRING ]

```

Then both GNU readelf and llvm-readelf print "E". I've added a test case to demonstrate.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1497
+    if (I != std::end(ElfSectionFlags)) {
+      Flags &= ~I->Value;
+      Str += I->AltName;
----------------
jhenderson wrote:
> It's entirely possible I'm misreading the code (I hate trying to follow bit-wise arithmetic), but isn't this line just a duplicate of the `Flags &= ~Flag;` above?
Right! I was expermenting with this code alot and seems forgot to remove this line.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71462/new/

https://reviews.llvm.org/D71462