Page MenuHomePhabricator

[llvm-readobj] Display sections that do not belong to a segment in the section-mapping
ClosedPublic

Authored by mattd on Feb 4 2019, 10:33 AM.

Details

Summary

The following patch adds the "None" line to the section to segment mapping dump.
That line lists the sections that do not belong to any segment.
I realize that this change differs from GNU readelf which does not display the latter information.

I'd rather not add this "feature" under a command line option. I think that might introduce confusion, since users would have to
make an additional decision as to if they want to see all of the section-to-segment map or just a subset of it.

Another option is to only print the "None" line if the --section-mapping option is passed; however,
that might also introduce some confusion, because the section-to-segment map would be different between`--program-headers`
and the --section-mapping output. While the difference is just the "None" line, it seems that if we choose to display
the segment-to-section mapping, then we should always display the whole map including the sections
that do not belong to segments.

Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .note.ABI-tag .gnu.hash 
   03     .init_array .fini_array .dynamic 
   04     .dynamic
   05     .note.ABI-tag
   06     .eh_frame_hdr
   07
   08     .init_array .fini_array .dynamic .got
   None   .comment .symtab .strtab .shstrtab <--- THIS LINE

Diff Detail

Repository
rL LLVM

Event Timeline

mattd created this revision.Feb 4 2019, 10:33 AM
mattd retitled this revision from [llvm-readobj] Display sections that do not belong to a segment in the section-to-segment dump. to [llvm-readobj] Display sections that do not belong to a segment in the segment-mapping.Feb 4 2019, 10:34 AM
mattd retitled this revision from [llvm-readobj] Display sections that do not belong to a segment in the segment-mapping to [llvm-readobj] Display sections that do not belong to a segment in the section-mapping.Feb 4 2019, 10:48 AM
mattd edited the summary of this revision. (Show Details)

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

llvm/tools/llvm-readobj/ELFDumper.cpp
3345 ↗(On Diff #185081)

I'd rename this variable to something else, either Sections, like above or NoSegmentSections or similar. The reason is to a void any potential confusion with Optional::None. The latter name is also slightly more self-documenting.

grimar added a comment.Feb 5 2019, 1:56 AM

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

My main concern is: is it ok we change the GNU style output?

Actually, I am confused and wonder why we have LLVM and GNU styles first of all.
I would understand if we would have GNU style that would try to be 1:1 to GNU and we would have LLVM style that would be an extension/restyling.

This patch changes GNU style of printSectionMapping. It makes me think we want LLVM style implementation of printSectionMapping instead maybe?

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

My main concern is: is it ok we change the GNU style output?

Actually, I am confused and wonder why we have LLVM and GNU styles first of all.
I would understand if we would have GNU style that would try to be 1:1 to GNU and we would have LLVM style that would be an extension/restyling.

Taken from D14128, which was the review to introduce the new style:

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

This patch changes GNU style of printSectionMapping. It makes me think we want LLVM style implementation of printSectionMapping instead maybe?

Certainly at some point we should add this to LLVM style, but I don't think it means we can't have it for GNU style too necessarily.

mattd marked an inline comment as done.Feb 5 2019, 9:40 AM

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

@jhenderson Thanks for tracking down that comment. I interpreted that comment as you described. Personally, I think a one line addition to the existing GNU style output is a reasonable change that will not detract users who might be expecting GNU-styled output. The change provides the user with the familiar GNU style, and provides a one line bonus (it provides a complete table (all of the sections are accounted for)). The main "issue" I see is that anyone diffing llvm-readelf output against the equivalent GNU output will see a line difference (llvm-readelf has the Notes, GNU readelf does not). If users diff output between llvm-readelf and GNU readelf, they would see other differences as well, primarily the number of digits used to represent some of the values in the --program-headers output.

rupprecht accepted this revision.Feb 5 2019, 11:52 AM

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

@jhenderson Thanks for tracking down that comment. I interpreted that comment as you described. Personally, I think a one line addition to the existing GNU style output is a reasonable change that will not detract users who might be expecting GNU-styled output. The change provides the user with the familiar GNU style, and provides a one line bonus (it provides a complete table (all of the sections are accounted for)). The main "issue" I see is that anyone diffing llvm-readelf output against the equivalent GNU output will see a line difference (llvm-readelf has the Notes, GNU readelf does not). If users diff output between llvm-readelf and GNU readelf, they would see other differences as well, primarily the number of digits used to represent some of the values in the --program-headers output.

I have the same interpretation. llvm-readelf should be possible to use as a replacement for readelf, but it may not be a drop-in replacement -- modifications to build scripts may be necessary to work around formatting differences. Also, I could come up with a long list of existing differences...

diff <(readelf -W -a llvm-readelf) <(llvm-readelf -a llvm-readelf) | wc -l
158116
This revision is now accepted and ready to land.Feb 5 2019, 11:52 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2019, 1:02 PM