[PATCH] D54124: [llvm-readelf] Make llvm-readelf more compatible with GNU readelf.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 6 08:37:52 PST 2018


jhenderson added a subscriber: edd.
jhenderson added a reviewer: jhenderson.
jhenderson added a comment.

Wow, that's some spooky timing. I didn't realise somebody else was thinking the exact same thing as me at the time! I brought this up on the mailing list earlier today, not realising that this review existed!

Some thoughts:

1. Why not break -s for both llvm-readobj and llvm-readelf? Yes, it will cause some test churn, but I think we should seriously consider avoiding weird lingering issues like this if we can.
2. What do you think about removing -t as an alias for --symbols in llvm-readelf (and possibly llvm-readobj)? -t means --section-details in GNU readelf, so has nothing to do with symbols. Of course, llvm-readelf doesn't (currently?) support --section-details, but maybe if we're breaking the meaning of switches we should get it completely right?
3. Are we comfortable breaking the behaviour for end-users with no prior warning? In my email, I proposed a few options, including "deprecating" old switch behaviour for a release, or simply providing a new switch that can be enabled by default in downstream ports to change the meaning of incompatible switches like -s.

I agree that changing to libOption seems sensible, but out of scope for this change. I also think we need to give careful consideration to the behaviour of single-dash options, since that's a difference in behaviour between GNU and LLVM tools (see for example . It's a common issue beyond llvm-readobj though (see also https://bugs.llvm.org/show_bug.cgi?id=31679).

I haven't looked at the actual change yet, but I'll try to do that tomorrow.


Repository:
  rL LLVM

https://reviews.llvm.org/D54124





More information about the llvm-commits mailing list