[PATCH] D60502: [llvm-nm] Add --special-syms

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 08:26:21 PDT 2019


jhenderson added a comment.

Okay, thinking about it, I think that adding the switch and doing nothing is probably an improvement, although I have some concerns about some people's expectations if we do provide the switch. I agree that some form of GNU mode is probably needed one way or the other, so this is fine as long as my other comments are addressed, so that users can have a path forward who want to migrate. I'd prefer a tool naming option, similar to what we have done in other tools rather than a switch naming option.

That being said, I don't think that the proposed symlink quite works as it stands (i.e. nm versus llvm-nm). In particular, in both llvm-readelf/readobj and llvm-symbolizer/llvm-addr2line, the name is different such that it is possible for the code to decide the behaviour by just looking for a partial match in its own name (e.g. "addr2line" versus "symbolizer" etc). I have seen examples of systems where the executable name format is essentially fixed (e.g. <platform>-<tool>-<version> where the <tool> part has to exactly match an LLVM executable's name), but who also want drop-in compatibility with GNU with only having to change the executable name they are using in their script. I don't have a clear solution to this though for the tools with GNU names currently, where we want to diverge.

In D60502#1479453 <https://reviews.llvm.org/D60502#1479453>, @jakehehrlich wrote:

> In D60502#1471512 <https://reviews.llvm.org/D60502#1471512>, @jhenderson wrote:
>
> > In D60502#1471500 <https://reviews.llvm.org/D60502#1471500>, @rupprecht wrote:
> >
> > > Just to see if I understand the comments so far: `--special-syms` is essentially already the default for `llvm-nm` (i.e. we print special symbols), but not for GNU `nm`. And so even though we print special symbols, a configure script that checks if `$NM --special-syms` is supported will fail when it shouldn't.
> > >
> > > If so, then LGTM -- this patch makes things less worse in all regards.
> >
> >
> > There's a counter-example to this though, which I'm concerned about. If we silently accept --special-syms, with it being the default always, there could be configure scripts that don't specify --special-syms that expect to NOT see those symbols (e.g. they're looking for all "normal" symbols). This in turn could result in a silent breakage.
>
>
> Those people weren't using llvm-nm apparently so we won't break them. If they were they'd already be broken. I think this strictly increases compatibility. Either way we have to choose who to break. People who might use llvm-nm in the future in place of nm and have written their script in a very specific way such that it needs to have those symbols filtered out or people who are using this flag today and are 100% broken but the flag not existing.


We have three options here:

1. Don't change anything. New users who expect the switch just have to remove it from their scripts, and will get a clear error message indicating what the problem is. Those who are not expecting the symbols to appear without the switch will be broken. Existing users of llvm-nm are unaffected.
2. Add the switch, do nothing. New users who require the switch's behaviour when enabled are happy. Those who are not expecting the symbols to appear without the switch will be broken. Existing users are unaffected. Clearly this is better than 1).
3. Add the switch doing the right thing, and change the default behaviour. New users who require the switch's behaviour when enabled are happy. Those who are not expecting the symbols to appear without the switch will also be happy. Existing users relying on the old default behaviour will be broken. Other existing users will be unaffected.

I don't agree that 2) is clearly better than 3), because I suspect the number of users of llvm-nm who rely on the different behaviour is likely quite small currently, whereas there are certainly many people who are transitioning or want to do so and we have no idea of their requirements. However, I cannot guarantee any of that. I only have the discussion from the BoF, where one point was that if we are going to break anything, do so now, and make sure there is a path forwards to work around the issue (in this case it would be to add the switch). 2) and 3) aren't strictly mutually exclusive (i.e. doing 2) before 3) is probably an option - both are better than 1).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60502/new/

https://reviews.llvm.org/D60502





More information about the llvm-commits mailing list