[PATCH] D71116: [test][tools] Add missing/Improve testing

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 03:55:24 PST 2019


jhenderson marked an inline comment as done.
jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-symbolizer/functions.s:17
 ## input address, and just prints it.
+# RUN: llvm-symbolizer 0 -f none --obj=%t.o | FileCheck %s --check-prefixes=LINKAGE,ERR
 # RUN: llvm-symbolizer 0 --functions none --obj=%t.o | FileCheck %s --check-prefixes=LINKAGE,ERR
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > MaskRay wrote:
> > > > Unrelated to this patch.
> > > > 
> > > > -f can be either --functions or --functions=, and the parsing accounts for the next option. This looks strange.
> > > I am not sure I understood your concern.
> > > It is just the same as `--functions none` but with the use of alias `-f`, isn't?
> > > 
> > > In another test we have a similar check:
> > > 
> > > ```
> > > # Check --obj aliases --exe, -e
> > > # RUN: llvm-symbolizer 0xa 0xb --exe=%t.o | FileCheck %s
> > > # RUN: llvm-symbolizer 0xa 0xb -e %t.o | FileCheck %s
> > > # RUN: llvm-symbolizer 0xa 0xb -e=%t.o | FileCheck %s
> > > # RUN: llvm-symbolizer 0xa 0xb -e%t.o | FileCheck %s
> > > ```
> > Like @grimar, I'm not sure what it is you are concerned by?
> > 
> > Note that `--functions none` is NOT the same as `--functions=none`. The former is actually equivalent to `--functions=linkage none`. This block of tests demonstrates this for both --functions and -f (which should be identical in behaviour).
> After re-reading the original comment today, I've got it I think.
> Fangrui did not mean that this change is unrelated, but pointed to the difference of the behavior,
> which is "--functions none is NOT the same as --functions=none". It is indeed looks strange
> (I've never saw such things before I think).
> 
Ah, that would make some sense. The problem basically is because --functions exists in GNU addr2line, but does not take an argument, whereas --functions was implemented in llvm-symbolizer to take an argument. To resolve this incompatibility with GNU, it was decided that `--functions arg` would function like GNU's `--functions`, i.e. not consume `arg` as part of the `--functions` option. See [[ https://github.com/llvm/llvm-project/commit/25ce596cd111ba93777338412ff9731b16f45cc6 | this commit ]]. I'd be reluctant to change the behaviour again now, even though we now have llvm-addr2line for GNU compatibility, since I don't think there's major benefit to breaking the behaviour for existing users. Note that we in particular need to be careful with something like `--functions 0x1234` to make sure that 0x1234 is treated as an address and not a value for `--functions`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71116





More information about the llvm-commits mailing list