Page MenuHomePhabricator

[llvm-symbolizer] Add support for specifying input addresses on the command-line
ClosedPublic

Authored by jhenderson on Thu, Jan 3, 8:11 AM.

Details

Summary

See https://bugs.llvm.org/show_bug.cgi?id=40070.

GNU addr2line accepts input addresses both on the command-line and via stdin. llvm-symbolizer previously only supported the latter. This change adds support for the former. As with addr2line, the new behaviour is to only look for addresses on stdin if no positional arguments were provided to llvm-symbolizer.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Thu, Jan 3, 8:11 AM
ruiu added inline comments.Wed, Jan 9, 9:01 AM
test/tools/llvm-symbolizer/basic.s
11–13 ↗(On Diff #180080)

Why don't you do

echo "0xa 0xb" | llvm-symbolizer ...

?

tools/llvm-symbolizer/llvm-symbolizer.cpp
202–204 ↗(On Diff #180080)

Now it is more straightforward to move fgets inside the while condition?

jhenderson updated this revision to Diff 180866.Wed, Jan 9, 9:39 AM
jhenderson marked 4 inline comments as done.

Move fgets inside loop termination condition.

Thanks @ruiu!

test/tools/llvm-symbolizer/basic.s
11–13 ↗(On Diff #180080)

When fed in via stdin, each input needs to be on a new line. I'm not sure I know a way to do this with a single echo command.

tools/llvm-symbolizer/llvm-symbolizer.cpp
202–204 ↗(On Diff #180080)

I'm actually not sure why this couldn't have been done before.

ruiu accepted this revision.Wed, Jan 9, 9:47 AM

LGTM

test/tools/llvm-symbolizer/basic.s
11–13 ↗(On Diff #180080)

In lld we do like this:

echo -e "0xa\n0xb\n" | ...
This revision is now accepted and ready to land.Wed, Jan 9, 9:47 AM

Use -e to simplify the echo commands in the test.

I'll commit this tomorrow if there are no objections before then, as I'm just leaving the office.

jhenderson marked an inline comment as done.Wed, Jan 9, 10:00 AM
ruiu added inline comments.Wed, Jan 9, 10:04 AM
test/tools/llvm-symbolizer/basic.s
11 ↗(On Diff #180868)

Not sure if it is safe without "". Perhaps you should add ""to be on the safe side.

12 ↗(On Diff #180868)

Remove %t2.input.

jhenderson marked 4 inline comments as done.

Add quotes and remove spurious redirection.

This revision was automatically updated to reflect the committed changes.
jhenderson added inline comments.Thu, Jan 10, 6:14 AM
test/tools/llvm-symbolizer/basic.s
11 ↗(On Diff #180868)

I think it probably is safe, but I'll add them anyway.

12 ↗(On Diff #180868)

Oops, thanks.