[PATCH] D60067: [llvm-symbolizer] Add llvm-addr2line.

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added inline comments.
This revision now requires changes to proceed.


================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:230
     Printer << (error(ResOrErr) ? DIInliningInfo() : ResOrErr.get());
+  } else if (IsAddr2Line) {
+    // With ClPrintFunctions == FunctionNameKind::LinkageName (default)
----------------
ikudrin wrote:
> rupprecht wrote:
> > I think this should be `else if (ClOutputStyle == OutputStyle::GNU)`, if someone wants to use `llvm-symbolizer` directly but still have GNU addr2line compatability.
> I'm not sure. Output style should define the way the information is displayed, but not affect the information itself. It'd be a bit unexpected, I think.
I actually agree with Jordan, so I'm retracting my LGTM.

We should try to have a consistent strategy between the tools. llvm-readobj already has an --elf-output-style switch, which switches the printing style between LLVM and GNU (and could be used for others in the future). llvm-readelf changes the default of that switch, but is otherwise essentially identical. There are other differences to do with command-line parsing, that are solely driven by the tool name.

The same should exist for llvm-symbolizer/llvm-addr2line. The tool's output style should be configured via the switch, with defaults of LLVM and GNU respectively. In some cases with might allow overriding of some behaviours. The command-line behaviour will still be driven by the tool name.


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

https://reviews.llvm.org/D60067