[PATCH] [llvm-symbolizer] Introduce the -dsym-hint option.

Alexey Samsonov vonosmas at gmail.com
Thu Oct 9 19:11:34 PDT 2014


Sorry, wasn't able to carefully review it today. Some early feedback.

Note that you will need tests in test/DebugInfo/ (add actual .dSYM bundles and macho files you're trying to symbolize under test/DebugInfo/, preferrably with sources and comments describing how to build them).

================
Comment at: LLVMSymbolize.cpp:215
@@ -213,1 +214,3 @@
+// /path/to/foo.dSYM/Contents/Resources/DWARF/foo.
 static std::string getDarwinDWARFResourceForPath(const std::string &Path) {
+  SmallString<16> DsymDirectory;
----------------
Why all this complexity? Can you just change this function to take .dSYM bundle, and call it either on file from .dSYM hints argument, or from PathToExe + ".dSYM"?

================
Comment at: LLVMSymbolize.cpp:329
@@ +328,3 @@
+
+void LLVMSymbolizer::populateDsymPaths(std::vector<std::string> &Paths, const std::string &ExePath) {
+  const std::string &ResourcePath =
----------------
This can be a static helper, pass Opts.DynamicHints here.
Output parameters ("Paths") should better go last in argument list.

================
Comment at: llvm-symbolizer.cpp:34
@@ -33,1 +33,3 @@
 static cl::opt<bool>
+ClVerbose("v", cl::init(false), cl::desc("Verbose mode"));
+
----------------
Please update docs/CommandGuide/llvm-symbolizer.rst

================
Comment at: llvm-symbolizer.cpp:130
@@ -121,2 +129,3 @@
                                ClPrintInlining, ClDemangle, ClDefaultArch);
+  Opts.InitDsymHints(ClDsymHint.begin(), ClDsymHint.end());
   LLVMSymbolizer Symbolizer(Opts);
----------------
Additional method looks like an overkill. Opts is a struct, you can just call
  Opts.DsymHints.assign(ClDsymHint.begin(), ClDsymHint.end());

http://reviews.llvm.org/D5705






More information about the llvm-commits mailing list