This is an archive of the discontinued LLVM Phabricator instance.

[llvm-symbolizer] Switch to using native symbolizer by default on Windows
ClosedPublic

Authored by akhuang on Nov 19 2020, 12:02 PM.

Details

Summary

llvm-symbolizer used to use the DIA SDK for symbolization on
Windows; this patch switches to using native symbolization, which was
implemented recently.

Users can still make the symbolizer use DIA by adding the -dia flag
in the LLVM_SYMBOLIZER_OPTS environment variable.

Diff Detail

Event Timeline

akhuang created this revision.Nov 19 2020, 12:02 PM
akhuang requested review of this revision.Nov 19 2020, 12:02 PM

I added two suggested edits for your consideration.

Other than those, this looks good.

(It seems there's some old Cmake cruft around LLVM_ENABLE_DIA_SDK that could probably be cleaned up in a separate patch.)

llvm/tools/llvm-symbolizer/Opts.td
44

I think this help string might be a smidge more useful to people who don't already know what DIA is.

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
291–298

--use-native-pdb-reader was always possible, but --dia isn't, so let's let the user know if we cannot satisfy their request.

jhenderson added inline comments.Nov 20 2020, 12:20 AM
llvm/tools/llvm-symbolizer/Opts.td
44

Could you please also add this option to the llvm-symbolizer Command Guide (llvm/docs/CommadnGuide/llvm-symbolizer.rst)? You might want to add a new subsection to the existing document for Windows/COFF/PDB specific options (like there is already for Mach-O).

akhuang updated this revision to Diff 306760.Nov 20 2020, 12:24 PM
akhuang marked 3 inline comments as done.

address comments

This revision is now accepted and ready to land.Nov 20 2020, 1:22 PM
jhenderson accepted this revision.Nov 23 2020, 12:04 AM

LGTM, with the suggestion.

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
294

Use WithColor::defaultWarningHandler instead and don't end the message with '.' (see the coding guidelines).

(I'm aware that llvm-symbolizer isn't consistent in this area, but we can work on improving it gradually)

akhuang updated this revision to Diff 307135.Nov 23 2020, 11:31 AM

Use WithColor::warning() to print the warning

This revision was landed with ongoing or failed builds.Nov 23 2020, 3:57 PM
This revision was automatically updated to reflect the committed changes.