Page MenuHomePhabricator

[llvm-symbolizer] Add back --use-symbol-table=true
ClosedPublic

Authored by MaskRay on Wed, Sep 2, 6:11 PM.

Details

Summary

It is used by clusterfuzz (https://github.com/google/clusterfuzz/pull/2009/)
and having this compatibility option for a while can help they do bisection
with the latest llvm-symbolizer.

Diff Detail

Event Timeline

MaskRay created this revision.Wed, Sep 2, 6:11 PM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay requested review of this revision.Wed, Sep 2, 6:11 PM
MaskRay updated this revision to Diff 289618.Wed, Sep 2, 6:22 PM

Add a test

MaskRay added inline comments.
llvm/test/tools/llvm-symbolizer/use-symbol-table.s
7

Cc @srhines: ndk-stack.py should probably remove --use-symbol-table=true

jhenderson accepted this revision.Thu, Sep 3, 12:47 AM

LGTM. Maybe the test name should be use-symbol-table-true.s? Do you plan to add more --use-symbol-table tests (there are currently no others)?

This revision is now accepted and ready to land.Thu, Sep 3, 12:47 AM

Do you plan to add more --use-symbol-table tests (there are currently no others)?

Never mind this question - I hadn't registered that the option was missing completely. Did --use-symbol-table=false (or some equivalent option) previously do something? If so, wemight want to add it back (with testing), as my understanding is that you didn't remove it deliberately.

Do you plan to add more --use-symbol-table tests (there are currently no others)?

Never mind this question - I hadn't registered that the option was missing completely. Did --use-symbol-table=false (or some equivalent option) previously do something? If so, wemight want to add it back (with testing), as my understanding is that you didn't remove it deliberately.

--use-symbol-table=false did something previously but I cannot find it in code search. --use-symbol-table=true was a strange option name but was used previously by asan_symbolize.py (in llvm-project) and elsewhere (I don't know why they used it). --use-symbol-table can override function names with .symtab information, which supposedly has at least some debugging meanings. We can bring it back when proper tests are added.

This revision was automatically updated to reflect the committed changes.
srhines added inline comments.Thu, Sep 3, 3:17 PM
llvm/test/tools/llvm-symbolizer/use-symbol-table.s
7

https://android-review.googlesource.com/c/platform/ndk/+/1419436 should remove this unnecessary option. Thanks for the heads up.

Dor1s added a comment.Wed, Sep 9, 1:21 PM

LGTM, thanks for adding this back!