Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvm-nm -U conflicts with GNU nm -U (since binutils>=2.38) #55297

Closed
MaskRay opened this issue May 6, 2022 · 9 comments
Closed

llvm-nm -U conflicts with GNU nm -U (since binutils>=2.38) #55297

MaskRay opened this issue May 6, 2022 · 9 comments

Comments

@MaskRay
Copy link
Member

MaskRay commented May 6, 2022

I noticed that GNU nm did not have -U and removed -U (--defined-only) from llvm-nm's --help and documentation in https://reviews.llvm.org/D105330 (like "soft deprecation", there is no warning invoking llvm-nm with the option).

Since binutils 2.38, GNU nm defines -U as an alias for --unicode, so we do have conflicting semantics now.

llvm-nm is a utility which tried to be compatible with both GNU nm and macOS nm. When the two nm implementations have conflicting semantics for an option, llvm-nm has a portability problem. I just did a bit research: for some older Mac OS X manpages, e.g. https://www.unix.com/man-page/osx/1/nm/ -U is not listed. On some newer systems, e.g. Apple M1, I can find the following in its nm(1):

-U Don't display undefined symbols

Note: the --unicode feature is probably not so useful right now.

@llvmbot
Copy link
Collaborator

llvmbot commented May 6, 2022

@llvm/issue-subscribers-tools-llvm-nm

@jh7370
Copy link
Collaborator

jh7370 commented May 6, 2022

llvm-nm's primary guarantee has always been comptability with GNU nm, not macOS. It's a bit unfortunate GNU decided to pick a clashing short alias, especially for a niche option, but I at least think we need to turn off its existing meaning now.

@compnerd
Copy link
Member

compnerd commented May 6, 2022

I think that we should conditionalize that on the platform. llvm-nm does provide a drop in replacement on Darwin as well, it would be unfortunate to just suddenly lose that.

@keith
Copy link
Member

keith commented May 6, 2022

It's possible that if -U is removed it will just cause more confusion since the Apple folks might just continue to include it in their fork to avoid breakages (not sure who from Apple should be cc'd for alignment?). If they did, it would lead to a strange discrepancy between upstream LLVM vs the distribution users get by default with Xcode. FWIW in case it's not clear nm on macOS is llvm-nm, there's a more up to date man page I mirror from Xcode here. If required I think handling that option only when on darwin platforms (or when targeting macho binaries?) is a fine compromise.

Ideally this issue can also be used to get some alignment on the level of compatibility that llvm-nm should strive for. This has come up before on an llvm-nm change of mine here. The resolution in that case was that @MaskRay put in the work (thanks for that!) to get the flag in binutils before we landed it in llvm-nm. There was also some discussion around not requiring binutils behavior mirroring for new long-form flags, but requiring it for new single letter flags, to reduce the chance of future conflicts. If this is the aligned upon opinion, is there some place we could codify this? Ideally reviewers of these tools would be on the same page, or there was somewhere we could at least link to when they aren't. I'm feeling thrash here since I added -W without any objection, likely because no one from this discussion reviewed my diff, and now a feature we're using in production is being deprecated because of this ambiguity. I know LLVM reviews potentially don't help here as specific developers may or may not see a diff they would object to, but ideally we could get as close as possible to consistent reviews so other contributors don't get hit by this same type of thrash in the future.

On that note, maybe given the niche use case of the unicode feature, this is also a place where llvm-nm could diverge instead?

FWIW at Lyft we have production code that uses both -U and -W which are both part of the deprecation diff linked here.

@kastiglione
Copy link
Contributor

llvm-nm's primary guarantee has always been comptability with GNU nm, not macOS.

@jh7370 is this discussed or documented somewhere?

@MaskRay
Copy link
Member Author

MaskRay commented May 8, 2022

I filed https://sourceware.org/pipermail/binutils/2022-May/120742.html whether GNU nm can redefine -U as --defined-only. I don't put a bet that they will do it, but if they do, it would end up with the best result.

Note that llvm-nm should not have different behaviors on different host platforms. That would make cross compilation inconvenient. I don't think llvm-nm should change its interpretation of options by inspecting input files.

If llvm-nm keeps having more GNU vs Darwin discrepancy, we may need a Darwin specific frontend (like llvm-objdump/llvm-otool, or llvm-libtool-darwin).

llvm-nm's primary guarantee has always been comptability with GNU nm, not macOS. It's a bit unfortunate GNU decided to pick a clashing short alias, especially for a niche option, but I at least think we need to turn off its existing meaning now.
@jh7370 is this discussed or documented somewhere?

By archaeology, GNU nm compatibility was the original goal and this generally reflects recent years' development.
However, in 2013 there were changes from Apple like 8da4bd6
that made -s use the Mac OS semantics for options with conflicting semantics.
I think the idea was to change llvm-nm so that it could replace classical Mac OS tools.

While I personally use -U as alias for --defined-only and find it convenient, I think changes like Add the -U flag to llvm-nm as an alias to -defined-only were unfortunate: they made unilateral decisions about short options for a tool with GNU nm compatibility (older Mac OS manpage not mentioning this option suggests that this usage was new). If they had informed GNU at that time, we would have ended with a better state.

@jh7370
Copy link
Collaborator

jh7370 commented May 9, 2022

+1 to the best resolution being getting GNU to change their option: if they do that, I don't think there's an issue with a temporary incompatibility, since our usage has been around for longer.

llvm-nm's primary guarantee has always been comptability with GNU nm, not macOS.

@jh7370 is this discussed or documented somewhere?

At the very least, it was discussed at at least one LLVM developer conference directly that I attended, in BoF or Round Table formats, and was indirectly talked about in other talks too. Here's the notes I have from the round table: https://lists.llvm.org/pipermail/llvm-dev/2019-April/132033.html and the BoF https://lists.llvm.org/pipermail/llvm-dev/2019-April/132032.html in Euro LLVM 2019. I don't think these notes are the only time, but I don't concretely recall what the previous occasions were, so trying to look them up may be a little tricky. Tools like llvm-nm are also listed in https://llvm.org/docs/CommandGuide/ under "GNU binutils replacements" too, though that's possibly a historical artifact because they were never originally intended to be Mac OS replacements. I feel like there have been more concrete discussions on mailing lists and reviews before, but I forget exact;y when.

One tool I can concretely say is that ELF llvm-objcopy was explicitly written to produce GNU-identical output, but only where that output made sense (I know this because I was involved from very early, basically day one, in its development).

I think we should take this discussion to Discourse, as it sounds like it needs to attract wider attention.

@keith
Copy link
Member

keith commented May 9, 2022

It looks like the binutils folks are open to using -U as it exist in llvm-nm today, and potentially adding -W/-no-weak as well (copying from the mailing list link above):

No, it could be changed. I have to say that I would have thought that -d
was a better choice for the short version. But anyway.

Before making the change however, I thought that it would be best to check
to see if there are any other inconsistencies and I did find a couple:

  • nm uses -s as the short version of --print-armap, whereas llvm-nm uses -M.
  • nm uses -V as the short version of --version, whereas llvm-nm does not have one.

The support/lack of -V does not strike me as important, and it should be a simple
matter to add -M as another short-version of --print-armap, so unless you have
any objections I will create a patch to change -U and add -M.

Cheers
Nick

PS. I also saw that llvm-nm has a -W/--no-weak option which is missing from nm.
Is there a binutils PR filed to add this feature ?

@MaskRay
Copy link
Member Author

MaskRay commented May 10, 2022

It looks like the binutils folks are open to using -U as it exist in llvm-nm today, and potentially adding -W/-no-weak as well (copying from the mailing list link above):

Thanks for the update. This will be the best outcome. I filed a GNU nm feature request for --no-weak/-W (https://sourceware.org/bugzilla/show_bug.cgi?id=29135).

We can then add back -U and -W to documentation.

I think llvm-nm -M should be removed. I mentioned that GNU nm should not add -M as another alias for --print-armap (https://sourceware.org/pipermail/binutils/2022-May/120766.html)

The only(?) conflicting option will be -s (llvm-nm: Mach-O's segment/section semantics; nm: --print-armap). For non-Mach-O object file formats, I suggest that -s should be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants