Page MenuHomePhabricator

Add -address option to symbolizer
ClosedPublic

Authored by khemant on Oct 7 2015, 10:23 AM.

Details

Summary

When llvm-symbolizer is passed a list of address, its nice to have an option which will print address corresponding line information. This improves human readability.

Diff Detail

Repository
rL LLVM

Event Timeline

khemant updated this revision to Diff 36755.Oct 7 2015, 10:23 AM
khemant retitled this revision from to Add -address option to symbolizer.
khemant updated this object.
khemant added a reviewer: llvm-commits.
khemant set the repository for this revision to rL LLVM.
samsonov edited edge metadata.Oct 8 2015, 3:11 PM

Please see http://llvm.org/docs/Phabricator.html on how to upload patches with more context.

I don't understand why you need to pass the value of "-address" option down to LLVMSymbolizer class - you can just handle it inside main(). I would also prefer option name to be more verbose like "-print-address".

samsonov added inline comments.Oct 8 2015, 3:13 PM
test/tools/llvm-symbolizer/addr.test
1 ↗(On Diff #36755)

Where did you get addr.exe from? I'd prefer to either use existing executable file, or commit the source file with description of build procedure. (the former is better).

khemant marked an inline comment as done.Oct 9 2015, 12:50 PM

You are right. I did not pay much attention to the driver.

khemant updated this revision to Diff 36980.Oct 9 2015, 12:58 PM
khemant edited edge metadata.

Option is handled inside main. Source is present as comment in test case.

samsonov accepted this revision.Oct 9 2015, 9:30 PM
samsonov edited edge metadata.

LGTM

test/tools/llvm-symbolizer/sym.test
18 ↗(On Diff #36980)

Please watch the Windows bots after the commit - you may need to relax slashes into "slash or backslash" here.

This revision is now accepted and ready to land.Oct 9 2015, 9:30 PM
This revision was automatically updated to reflect the committed changes.