This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] Accept -S as an alias for --sections
ClosedPublic

Authored by mcgrathr on Jun 3 2017, 4:17 PM.

Details

Summary

In GNU readelf, the short option for --sections is upper-case -S.

Note that GNU uses lower-case -s to mean --symbols, while LLVM
uses -s to mean --sections and -t to mean --symbols (-t has yet a
different meaning in GNU). So command-line uses with -S can now
be compatible, but uses with -s or -t are still incompatible.

Diff Detail

Repository
rL LLVM

Event Timeline

mcgrathr created this revision.Jun 3 2017, 4:17 PM
davide edited edge metadata.Jun 3 2017, 4:27 PM

So, I don't this this divergence from GNU readelf is a good thing in general (and it's probably just historical).
I'm not sure whether we should bite the bullet and change the options to be readelf compliant (unless there's a good reason fro not doing it).
The argument in favour is that llvm-readobj is supposed to be a developer tool only (for testing). The GNU compatibility mode has been added relatively recently so, probably few people are already using it. The argument against it is that people generally expect cmdline options to be stable ("cast in stone") over releases.

This change is orthogonal to the -s switch and I'd like to get it landed independent of decisions about -s.
I only brought it up to point out the limitations of compatibility with readelf even after these changes.
But these alone are incremental progress and cover all the uses cases for readelf in the build of the system I happen to be building.

davide added a comment.Jun 3 2017, 5:17 PM

This change is orthogonal to the -s switch and I'd like to get it landed independent of decisions about -s.
I only brought it up to point out the limitations of compatibility with readelf even after these changes.
But these alone are incremental progress and cover all the uses cases for readelf in the build of the system I happen to be building.

Yes, this one LGTM as well. My main concern stems from the divergence with GNU (with no good reason).
tl;dr: I'd like this to be discussed further (independently from this patch). Whether this should happen as part of this review (or on llvm-dev), I have no strong opinion about [but a slight preference for the latter].

phosek edited edge metadata.Jul 10 2017, 8:11 PM

Is it OK to land this change (and the entire stack) or do you want to have the discussion about GNU flag compatibility first?

This revision was automatically updated to reflect the committed changes.