Page MenuHomePhabricator

[llvm-readobj] Print section type values for unknown sections.
ClosedPublic

Authored by mattd on Feb 26 2019, 4:57 PM.

Details

Summary

This patch displays a hexadecimal section value (Elf_Shdr::sh_type) or section-relative offset when printing unknown sections.

Here is a subset of the output (ignoring the fields following "Type" when dumping an ELF's GNU --section-headers table).
Section Headers:

[Nr] Name              Type
[16] android_rel       LOOS+0x1
[17] android_rela      LOOS+0x2
[27] unknown           0x1000: <unknown>
[28] loos              LOOS+0
[30] hios              VERSYM
[31] loproc            LOPROC+0
[33] hiproc            LOPROC+0xFFFFFFF
[34] louser            LOUSER+0
[36] hiuser            LOUSER+0x7FFFFFFF

As a comparison, the previous output looked something like the above, but with a blank "Type" field:

[Nr] Name              Type
[27] unknown
[28] loos
[30] hios              VERSYM
[31] loproc
[33] hiproc
[34] louser
[36] hiuser

This fixes PR40773

Diff Detail

Repository
rL LLVM

Event Timeline

mattd created this revision.Feb 26 2019, 4:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2019, 4:57 PM
Bigcheese accepted this revision.Feb 26 2019, 5:01 PM
Bigcheese added a subscriber: Bigcheese.

lgtm

Should the +0 be +0x0 so that it's always a hex digit? Or is the +0 what the binutils already output?

This revision is now accepted and ready to land.Feb 26 2019, 5:01 PM
mattd added a comment.Feb 26 2019, 5:04 PM

lgtm

Should the +0 be +0x0 so that it's always a hex digit? Or is the +0 what the binutils already output?

Thanks for the quick review! GNU's readelf output's "+0" in this case, without the "0x" prefix.

Then I'm fine with the patch as is.

rupprecht accepted this revision.Feb 26 2019, 9:33 PM

Thanks!

MaskRay added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
2941 ↗(On Diff #188481)

How about folding SHT_LO{OS,PROC<USER}* cases into getSectionTypeOffsetString?

jhenderson requested changes to this revision.Feb 27 2019, 1:55 AM
jhenderson added inline comments.
llvm/test/tools/llvm-readobj/elf-section-types.test
103 ↗(On Diff #188481)

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.

115 ↗(On Diff #188481)

GNU_NEXT -> GNU-NEXT here onwards.

126 ↗(On Diff #188481)

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.

llvm/tools/llvm-readobj/ELFDumper.cpp
2941 ↗(On Diff #188481)

+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.

This revision now requires changes to proceed.Feb 27 2019, 1:55 AM
mattd updated this revision to Diff 188561.Feb 27 2019, 8:57 AM
mattd marked 5 inline comments as done.
  • Fixed the test case (removed underscore typo, and extraneous shstrtab test).
  • Emit ANDROID_REL ANDROID_RELA instead of section offsets.
  • Modified the range check, allowing us to remove some now redundant code for printing the lower-bound section names.
This revision is now accepted and ready to land.Feb 27 2019, 9:09 AM
This revision was automatically updated to reflect the committed changes.