[PATCH] D58701: [llvm-readobj] Print section type values for unknown sections.

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/test/tools/llvm-readobj/elf-section-types.test:103
 # GNU-NEXT: relr                    RELR
-## FIXME: These next two lines should print something like ANDROID_REL and ANDROID_RELA.
-## See https://bugs.llvm.org/show_bug.cgi?id=40773.
-# GNU-NEXT: android_rel                      0000000000000000
-# GNU-NEXT: android_rela                     0000000000000000
+# GNU-NEXT: android_rel             LOOS+0x1
+# GNU-NEXT: android_rela            LOOS+0x2
----------------
As these are well-known to LLVM values, it might be worth filing a bug and adding a TODO here to make them ANDROID_REL and ANDROID_RELA, similar to the original FIXME comment. Or just fix them in this patch.


================
Comment at: llvm/test/tools/llvm-readobj/elf-section-types.test:115
+# GNU-NEXT: unknown                 0x1000: <unknown>
+# GNU_NEXT: loos                    LOOS+0
+# GNU_NEXT: fooos                   LOOS+0xF00
----------------
GNU_NEXT -> GNU-NEXT here onwards.


================
Comment at: llvm/test/tools/llvm-readobj/elf-section-types.test:126
+# GNU_NEXT: .strtab                 STRTAB
+# GNU_NEXT: .shstrtab               STRTAB
 
----------------
FWIW, there's an odd mis-match here versus the LLVM testing, which doesn't test .shstrtab. When I wrote this test, I just wanted one of each section type, i.e. I didn't need to test both .strtab and .shstrtab. I don't mind both being there, but then you should also add .shstrtab to LLVM.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2941
     return "VERSYM";
+  case SHT_LOOS:
+    return "LOOS+0";
----------------
MaskRay wrote:
> How about folding `SHT_LO{OS,PROC<USER}*` cases into getSectionTypeOffsetString?
+1 to this, especially for SHT_HI*, since those are no different either way.

I don't think we should handle these types specially in this case, but we can if we are keen for exact GNU output identity. Personally, I'd just favour simpler code and let them print as `SHT_LOOS+0x0` etc.


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

https://reviews.llvm.org/D58701