llvm-nm: add flag to suppress no symbols warning

[keith]

Summary

This spelling matches binutils https://sourceware.org/bugzilla/show_bug.cgi?id=27408

[jhenderson]

Added a couple more people with experience with Darwin, to see if they have any thoughts on the option name/whether it would be helpful to be identical etc. Also added @grimar/@MaskRay/@rupprecht who have a lot of experience with the binutils too.

Please update the llvm-nm documentation found in the CommandGuide directory to include the new switch as part of this change.

I'm happy to support adding the option, if there's a desire for it. It's trivial enough that it seems reasonable. I don't know about the name of it, and would prefer a shorter name if anybody can come up with one (my only suggestion is --no-no-symbols-warning which is almost as verbose and possibly more confusing), but if matching the libtool name is useful, I'm not actively opposed to it.

llvm/test/tools/llvm-nm/X86/nm-no-symbols.test

14

If I'm right, llvm-nm should not print any output at all in this case? If that's the case, I'd change this line to: # NO-WARNING-NOT: {{.}} to make it a little more robust against possible further changes/typos etc.

[grimar]

llvm/test/tools/llvm-nm/X86/nm-no-symbols.test

13

-no-warning-for-no-symbols -> --no-warning-for-no-symbols

(there is an agreement to use double dash for long versions of options in tests.
E.g: use --reverse-sort instead of -reverse-sort or just use -r.
I think one of the reasons was to make the difference between things like --reverse-sort and -r everse-sort more obvious).

[MaskRay]

I cannot find any search result about no-warning-for-no-symbols. Is -no-warning-for-no-symbols really an existing option? libtool is an ar like tool.

Second, I wonder how you are going to plug -no-warning-for-no-symbols into a build system. If you only parse stdout, you can ignore stderr. Even if you do, you can probably use grep -v '^no symbols'. This will have better portability (supported on older nm, supported on other binary formats).

[rupprecht]

In D83152#2133855, @MaskRay wrote:

I cannot find any search result about no-warning-for-no-symbols. Is -no-warning-for-no-symbols really an existing option? libtool is an ar like tool.

I found it by looking for underscores instead of hyphens: -no_warning_for_no_symbols.
However, the flag is an ar/ranlib/libtool flag, not nm, AFAICT.

Second, I wonder how you are going to plug -no-warning-for-no-symbols into a build system. If you only parse stdout, you can ignore stderr. Even if you do, you can probably use grep -v '^no symbols'. This will have better portability (supported on older nm, supported on other binary formats).

I agree this is likely the simpler option (just add 2> /dev/null to the build script using nm)

[keith]

In D83152#2133950, @rupprecht wrote:

In D83152#2133855, @MaskRay wrote:

I cannot find any search result about no-warning-for-no-symbols. Is -no-warning-for-no-symbols really an existing option? libtool is an ar like tool.

I found it by looking for underscores instead of hyphens: -no_warning_for_no_symbols.
However, the flag is an ar/ranlib/libtool flag, not nm, AFAICT.

Yea sorry I should have been more clear, it's not the _exact_ same spelling because of the conventions used in nm with - instead of _.

Second, I wonder how you are going to plug -no-warning-for-no-symbols into a build system. If you only parse stdout, you can ignore stderr. Even if you do, you can probably use grep -v '^no symbols'. This will have better portability (supported on older nm, supported on other binary formats).

I agree this is likely the simpler option (just add 2> /dev/null to the build script using nm)

If folks feel strongly about this that would definitely work, this felt like a safer way to silence this for the future for me, but if you all think it's not worth adding an option for that's fine.

[jhenderson]

In D83152#2134405, @keith wrote:

In D83152#2133950, @rupprecht wrote:

In D83152#2133855, @MaskRay wrote:

I cannot find any search result about no-warning-for-no-symbols. Is -no-warning-for-no-symbols really an existing option? libtool is an ar like tool.

I found it by looking for underscores instead of hyphens: -no_warning_for_no_symbols.
However, the flag is an ar/ranlib/libtool flag, not nm, AFAICT.

Yea sorry I should have been more clear, it's not the _exact_ same spelling because of the conventions used in nm with - instead of _.

Second, I wonder how you are going to plug -no-warning-for-no-symbols into a build system. If you only parse stdout, you can ignore stderr. Even if you do, you can probably use grep -v '^no symbols'. This will have better portability (supported on older nm, supported on other binary formats).

I agree this is likely the simpler option (just add 2> /dev/null to the build script using nm)

If folks feel strongly about this that would definitely work, this felt like a safer way to silence this for the future for me, but if you all think it's not worth adding an option for that's fine.

I don't have strong opinions either way. I think there probably is some benefit to adding the new option: one counter-point to redirecting stderr to /dev/null is that will hide any real error messages llvm-nm (e.g. caused by a missing input file). I don't think that's a good idea personally. That being said, if stderr is being redirected somewhere other than /dev/null maybe it's okay. Another issue is that because stderr and stdout are often handled independently, but end up in the same final output, you can end up with the "no symbols" message appearing quite some way from where it really belongs, which could be slightly confusing. However, it's not an issue I actually have at the moment.

Another possible benefit (again not something I personally have worried about, but one I thought of) is that users could specify the option to avoid having to special case parsing scripts to handle no symbols. A naive script would typically just iterate through the lines and expect each one to correspond to a symbol, so if there are none, the parser might break.

Another option name could just be --quiet, although there's a small risk that could clash in the future with a GNU option.

[rupprecht]

In D83152#2135334, @jhenderson wrote:

In D83152#2134405, @keith wrote:

In D83152#2133950, @rupprecht wrote:

In D83152#2133855, @MaskRay wrote:

I cannot find any search result about no-warning-for-no-symbols. Is -no-warning-for-no-symbols really an existing option? libtool is an ar like tool.

I found it by looking for underscores instead of hyphens: -no_warning_for_no_symbols.
However, the flag is an ar/ranlib/libtool flag, not nm, AFAICT.

Yea sorry I should have been more clear, it's not the _exact_ same spelling because of the conventions used in nm with - instead of _.

Second, I wonder how you are going to plug -no-warning-for-no-symbols into a build system. If you only parse stdout, you can ignore stderr. Even if you do, you can probably use grep -v '^no symbols'. This will have better portability (supported on older nm, supported on other binary formats).

I agree this is likely the simpler option (just add 2> /dev/null to the build script using nm)

If folks feel strongly about this that would definitely work, this felt like a safer way to silence this for the future for me, but if you all think it's not worth adding an option for that's fine.

I don't have strong opinions either way. I think there probably is some benefit to adding the new option: one counter-point to redirecting stderr to /dev/null is that will hide any real error messages llvm-nm (e.g. caused by a missing input file). I don't think that's a good idea personally.

The return code would be 1 for a missing input file, which should fail whatever build step/etc. is executing llvm-nm. If you're ignoring that return code, you have other problems :)

That said, I'm on the fence here -- it doesn't seem all that complex to add it, but it also doesn't seem worth the extra (minor) complexity. Maybe I don't understand the use case well enough. Is there a build step executing llvm-nm that chokes on this? Can you say more about the use case?

That being said, if stderr is being redirected somewhere other than /dev/null maybe it's okay. Another issue is that because stderr and stdout are often handled independently, but end up in the same final output, you can end up with the "no symbols" message appearing quite some way from where it really belongs, which could be slightly confusing. However, it's not an issue I actually have at the moment.

Another possible benefit (again not something I personally have worried about, but one I thought of) is that users could specify the option to avoid having to special case parsing scripts to handle no symbols. A naive script would typically just iterate through the lines and expect each one to correspond to a symbol, so if there are none, the parser might break.

Since "no symbols" is printed to stderr, not stdout, you would have to go out of your way to have such a script need to handle "no symbols" as input by joining stdout and stderr. Normal pipelining ($ llvm-nm foo.o | script) would drop stderr.

Another option name could just be --quiet, although there's a small risk that could clash in the future with a GNU option.

[keith]

Is there a build step executing llvm-nm that chokes on this? Can you say more about the use case?

Our use case for this is just scripts that analyze the symbols from some binaries, separately from our build system. Theoretically we could drop stderr (since we do handle the error code) and that might be "good enough". I would be a _bit_ worried without reading the rest of the code here whether by dropping stderr would lose any other "more-important" warnings (I might also be worried about this if we had a more generic --quiet option). I see this warning as pretty noisy for large static library targets where having object files with no symbols based on your deployment target is often expected in the libraries we're using.

[MaskRay]

(So that it can leave people's review queue)

[keith]

Update style in test

[MaskRay]

Sorry, I keep thinking this is not worthwhile because the advantage does not justify a new option no-warning-for-no-symbols. If you think it is justifiable, coordination on [e-mail] and making them accept an option as well is way to proceed here.

[keith]

thanks for the review

[kastiglione]

@MaskRay I haven't heard of coordination with binutils, can you say more about that? I'm particularly curious because llvm-nm, and other llvm tools, have many options not present in binutils. As an example @keith added --no-weak previously.

How do flags get justified? I would accept this, because the complexity is so low (a bool used in one place), and non-breaking. But who am I to decide :)

[MaskRay]

In D83152#2551777, @kastiglione wrote:

@MaskRay I haven't heard of coordination with binutils, can you say more about that? I'm particularly curious because llvm-nm, and other llvm tools, have many options not present in binutils.

In recent years I think we are more careful about documentation/tests and require justification for options.

As an example @keith added --no-weak previously.

The name --no-weak is fine. What I'd ask is the rationale for the new option: why is there a need to list non-weak symbols?
A potential risk with D48751 is that if GNU nm uses -W for other purposes, we may have a trouble.

Another example: llvm-nm -s has been taken for segment section. In nm, it means --print-armap.

How do flags get justified? I would accept this, because the complexity is so low (a bool used in one place), and non-breaking. But who am I to decide :)

I think it is case by case. For this one, 2> /dev/null is a simple and easy workaround. I find it difficult to argue against this choice. There was an equally good suggestion --quiet we cannot refute.

When there is doubt, it seems a good idea if the author is willing to engage with the other implementation. The adoption can serve as a tiebreak.

[keith]

I think it is case by case. For this one, 2> /dev/null is a simple and easy workaround. I find it difficult to argue against this choice. There was an equally good suggestion --quiet we cannot refute.

I disagree that this is a viable workaround since nm produces quite a few more errors that you likely don't want to filter out

[jhenderson]

In D83152#2553151, @keith wrote:

I think it is case by case. For this one, 2> /dev/null is a simple and easy workaround. I find it difficult to argue against this choice. There was an equally good suggestion --quiet we cannot refute.

I disagree that this is a viable workaround since nm produces quite a few more errors that you likely don't want to filter out

+1 to this - in fact, I already pointed this out further up in the comments.

I am strongly opposed to us having to get buy-in from GNU binutils to be able to add new options - I don't think has ever been agreed upon widely. Requiring this coordination stifles addition of new features in our toolchain. Over the years, we have added many options and features to the LLVM binutils that diverge from what GNU have done. As long as they don't clash, and have a valid use case, I don't think this is a bad thing - if GNU developers want to later implement it themselves, there's nothing stopping them, but I don't think we should be forced into coordination on it. The one thing I would say is that we should avoid using option names that have the potential to clash, for example single character aliases, unless there's some coordination.

[MaskRay]

In D83152#2553446, @jhenderson wrote:

In D83152#2553151, @keith wrote:

I think it is case by case. For this one, 2> /dev/null is a simple and easy workaround. I find it difficult to argue against this choice. There was an equally good suggestion --quiet we cannot refute.

I disagree that this is a viable workaround since nm produces quite a few more errors that you likely don't want to filter out

+1 to this - in fact, I already pointed this out further up in the comments.

I am strongly opposed to us having to get buy-in from GNU binutils to be able to add new options - I don't think has ever been agreed upon widely. Requiring this coordination stifles addition of new features in our toolchain. Over the years, we have added many options and features to the LLVM binutils that diverge from what GNU have done. As long as they don't clash, and have a valid use case, I don't think this is a bad thing - if GNU developers want to later implement it themselves, there's nothing stopping them, but I don't think we should be forced into coordination on it. The one thing I would say is that we should avoid using option names that have the potential to clash, for example single character aliases, unless there's some coordination.

I have expressed my opinion clearly:

I think it is case by case. For this one, 2> /dev/null is a simple and easy workaround. I find it difficult to argue against this choice. There was an equally good suggestion --quiet we cannot refute.

In general I don't object to new LLVM specific options, but that should be useful and try avoiding potential clash with GNU. no symbols is an area where the GNU behavior may want to improve (you can dig up the history of our no symbols support - it is complex), so I believe this is an area where they'd like to have an opinion as well.
I don't see how --no-warning-for-no-symbols is better than --quiet, but --quiet is an option name they could use for other purposes.

You can also filter out no symbols and keep other warnings.

[kastiglione]

What, if any, is the community opinion on hidden flag use? Can this be a hidden flag and be done with it? I assume means hidden means no forward guarantees, rely on it at your own discretion.

I don't see how --no-warning-for-no-symbols is better than --quiet, but --quiet is an option name they could use for other purposes.

seems like you answer your own question :)

[jhenderson]

In D83152#2555229, @kastiglione wrote:

What, if any, is the community opinion on hidden flag use? Can this be a hidden flag and be done with it? I assume means hidden means no forward guarantees, rely on it at your own discretion.

My opinion: in this context, I don't think it buys us enough, but I'm not opposed to hidden flags in general. I don't know what the LLVM policy on hidden flags generally is though - more often than not, I've found hidden flags to be hidden accidentally!

I don't see how --no-warning-for-no-symbols is better than --quiet, but --quiet is an option name they could use for other purposes.

seems like you answer your own question :)

Exactly: if --quiet has the risk of being used for something else, we should avoid using it (just the same as for short single-letter aliases). If on the other hand, someone wants to coordinate with GNU and/or they happen to implement --quiet to cover this case, we can remove the --no-warning-for-no-symbols option at that point, and direct users towards --quiet instead. I don't think it should be a requirement to do this coordination, especially given how easy it would be to migrate in this case.

[MaskRay]

In D83152#2556189, @jhenderson wrote:

In D83152#2555229, @kastiglione wrote:

What, if any, is the community opinion on hidden flag use? Can this be a hidden flag and be done with it? I assume means hidden means no forward guarantees, rely on it at your own discretion.

My opinion: in this context, I don't think it buys us enough, but I'm not opposed to hidden flags in general. I don't know what the LLVM policy on hidden flags generally is though - more often than not, I've found hidden flags to be hidden accidentally!

I don't see how --no-warning-for-no-symbols is better than --quiet, but --quiet is an option name they could use for other purposes.

seems like you answer your own question :)

Exactly: if --quiet has the risk of being used for something else, we should avoid using it (just the same as for short single-letter aliases). If on the other hand, someone wants to coordinate with GNU and/or they happen to implement --quiet to cover this case, we can remove the --no-warning-for-no-symbols option at that point, and direct users towards --quiet instead. I don't think it should be a requirement to do this coordination, especially given how easy it would be to migrate in this case.

I don't agree. If the coordination is done first, we have a potential to use --quiet to right away. Then we don't need to have a temporary --no-warning-for-no-symbols which will eventually be removed (and potential to break users).
The no symbols has caused us lots of trouble (see the history). I imagined that binutils folks would have some opinions on it. Given the contentious previous discussions, I think it is entirely justified to ask them for an opinion.
I have said in some other reviews, their objection does not necessarily block us to add an option, as we could get a promise that they will not add an option to deliberately make us incompatible.

[jhenderson]

In D83152#2557844, @MaskRay wrote:

I don't agree. If the coordination is done first, we have a potential to use --quiet to right away. Then we don't need to have a temporary --no-warning-for-no-symbols which will eventually be removed (and potential to break users).

Whilst what you are saying is entirely true, the issue is that it unnecessarily (in my opinion) raises the bar on a contribution that is already small and quite simple. Not all developers know or care how to discuss things with the GNU community. Saying that they do makes it more likely that they will simply walk away from contributing what might otherwise be a useful patch. In this case, it's clearly not a code quality issue, nor is it a tricky thing for us to change in the future should some future developer decide they want to discuss the option with GNU/GNU independently go their own way.

The no symbols has caused us lots of trouble (see the history). I imagined that binutils folks would have some opinions on it. Given the contentious previous discussions, I think it is entirely justified to ask them for an opinion.

Could you point out where these contentious previous discussions were? I looked back through the history, and there didn't seem to be any real issues - it was just about getting the llvm-nm implementation right so that it matches the GNU behaviour, but it's quite possible I missed something, or I might just be misunderstanding you. I don't think difficulties trying to get our implementation to match GNU's are really relevant to this patch, since this is about a new option, so won't impact the default behaviour.

[kastiglione]

@jhenderson I'm glad you brought up the bar to contribution, because it crossed my mind too. It seems most features added to llvm tools did not need to coordinate with binutils.

nor is it a tricky thing for us to change in the future

👍

[MaskRay]

You can use --quiet instead. They accepted my --quiet patch: https://sourceware.org/bugzilla/show_bug.cgi?id=27408

In D83152#2559118, @jhenderson wrote:

In D83152#2557844, @MaskRay wrote:

I don't agree. If the coordination is done first, we have a potential to use --quiet to right away. Then we don't need to have a temporary --no-warning-for-no-symbols which will eventually be removed (and potential to break users).

Whilst what you are saying is entirely true, the issue is that it unnecessarily (in my opinion) raises the bar on a contribution that is already small and quite simple. Not all developers know or care how to discuss things with the GNU community. Saying that they do makes it more likely that they will simply walk away from contributing what might otherwise be a useful patch. In this case, it's clearly not a code quality issue, nor is it a tricky thing for us to change in the future should some future developer decide they want to discuss the option with GNU/GNU independently go their own way.

Discussing with binutils is case by case. This feature solves a minor insurmountable problem.
("no symbols" can be easily filtered out in the log.)
--quiet is a common option which folks may want to have an opinion on.
We would have migration cost that adds a different option first, adds --quiet for compatibility, then drops the first option.

The no symbols has caused us lots of trouble (see the history). I imagined that binutils folks would have some opinions on it. Given the contentious previous discussions, I think it is entirely justified to ask them for an opinion.

Could you point out where these contentious previous discussions were? I looked back through the history, and there didn't seem to be any real issues - it was just about getting the llvm-nm implementation right so that it matches the GNU behaviour, but it's quite possible I missed something, or I might just be misunderstanding you. I don't think difficulties trying to get our implementation to match GNU's are really relevant to this patch, since this is about a new option, so won't impact the default behaviour.

We added no symbols, then dropped the diagnostic if there is hidden symbols (e.g. ARM mapping symbols $a which are normally not display for arm-*-nm), then changed the output stream from stdout to stderr.

[keith]

Update to --quiet spelling

[keith]

Thanks for driving that! Updated my patch to use --quiet. I did hesitate a bit on specifically saying 'no symbols' in the help string, but I guess we can update that in the future if anything else makes sense to gate with this flag as well.

[keith]

Add flag to test

[jhenderson]

You need to add the --quiet option to the llvm-nm documentation page, which you can find at llvm/docs/CommandGuide/llvm-nm.rst.

llvm/test/tools/llvm-nm/X86/nm-no-symbols.test

13

I should have noticed this before. Could you replace the FileCheck line with count 0 instead? That'll be simpler, I think.

llvm/tools/llvm-nm/llvm-nm.cpp

158–159

I don't think these options are in any particular order, but it would probably make sense not to split up the two size-related options! I think you can just move this somewhere else.

[keith]

Update test and docs

[MaskRay]

llvm/docs/CommandGuide/llvm-nm.rst

209 ↗

(On Diff #328622)

[keith]

Update help

[keith]

Thanks again everyone!

llvm/test/tools/llvm-nm/X86/nm-no-symbols.test

 # RUN: yaml2obj %s -o %t.o
 # RUN: llvm-nm %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 
 # CHECK: [[FILE]]: no symbols{{$}}
+
+# RUN: llvm-nm --quiet %t.o 2>&1 | FileCheck %s -DFILE=%t.o --check-prefix=NO-WARNING --allow-empty
+# NO-WARNING-NOT: {{.}}

llvm/tools/llvm-nm/llvm-nm.cpp

 
 cl::opt<bool> PrintSize("print-size",
                         cl::desc("Show symbol size as well as address"),
                         cl::cat(NMCat));
 cl::alias PrintSizeS("S", cl::desc("Alias for --print-size"),
                      cl::aliasopt(PrintSize), cl::Grouping);
 bool MachOPrintSizeWarning = false;
 
+cl::opt<bool> Quiet("quiet", cl::desc("Suppress 'no symbols' diagnostic"),
+                    cl::cat(NMCat));
+
 cl::opt<bool> SizeSort("size-sort", cl::desc("Sort symbols by size"),
                        cl::cat(NMCat));
 
 cl::opt<bool> WithoutAliases("without-aliases", cl::Hidden,
                              cl::desc("Exclude aliases from output"),
                              cl::cat(NMCat));
 
 cl::opt<bool> ArchiveMap("print-armap", cl::desc("Print the archive map"),
static void dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
   // This is needed if strip(1) -T is run on a binary containing swift
   // language symbols for example.  The option -only-dyldinfo will fake up
   // all symbols from the dyld export trie as well as the bind info.
   if (MachO && !NoDyldInfo)
     dumpSymbolsFromDLInfoMachO(*MachO);
 
   CurrentFilename = Obj.getFileName();
 
-  if (Symbols.empty() && SymbolList.empty()) {
+  if (Symbols.empty() && SymbolList.empty() && !Quiet) {
     writeFileName(errs(), ArchiveName, ArchitectureName);
     errs() << "no symbols\n";
   }
 
   sortAndPrintSymbolList(Obj, printName, ArchiveName, ArchitectureName);
 }
 
 // checkMachOAndArchFlags() checks to see if the SymbolicFile is a Mach-O file