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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D60067#1462784 <https://reviews.llvm.org/D60067#1462784>, @lebedev.ri wrote:

> I guess it's not considered a common practice, but i think it would be a really good idea to add a docs page for the new tool at the same time. Else it will be forgotten, much like for half of other tools..


I agree, although it's worth noting that llvm-symbolizer DOES have this documentation, so it may be worth piggy-backing on that rather than writing an entirely new doc.

Overall, I'm basically happy with this change. One of the major points brought up at the BoF and round table for LLVM binutils at the just-gone Euro LLVM Developers' Meeting was that we might want to consider a machine-readable output style in the future. If we do that, we will likely need to refactor how this all works (i.e. how LLVM style, GNU style and machine-readable style get printed), but that's probably beyond the scope of this change. Getting the new tool in allows people to start using it now.



================
Comment at: test/tools/llvm-symbolizer/sym.test:64
+#A2L-NEXT:    {{[/\]+}}tmp{{[/\]+}}x.c:3
+#A2L_FI-NEXT: inc
+#A2L_I-NEXT:  {{[/\]+}}tmp{{[/\]+}}x.c:7
----------------
You should add a {{$}} to the end of this line as it would also match "inctwo".


================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:236
+    // behavior of addr2line. Symbolizer.symbolizeInlinedCode() overrides only
+    // the topmost function, which suites our needs better.
+    auto ResOrErr = Symbolizer.symbolizeInlinedCode(
----------------
suites -> suits


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

https://reviews.llvm.org/D60067