[PATCH] D118633: [Symbolizer] Add Build ID flag to llvm-symbolizer.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-symbolizer.rst:189
+  Look up the object using the given build ID, specified as a hexadecimal
+  string. The --obj family of flags takes precedence.
+
----------------
phosek wrote:
> I wonder whether it wouldn't be less error prone to diagnose the case when user uses both `--obj` and `--build-id` as an error? 
+1: I think it should be an error if a user specifies both, unless you have a good use-case for it.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:308
 }
 
+} // end anonymous namespace
----------------
Don't think you want this blank line.


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:369-372
+    if (!Symbolizer.findDebugBinary(BuildID, BinaryName)) {
+      errs() << "Could not find build ID: '" << toHex(BuildID) << "'\n";
+      exit(1);
+    }
----------------
This is untested.

Also, the error format is not consistent with the LLVM coding standards or what llvm-symbolizer prints when there's a missing file, as far as I can see.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118633/new/

https://reviews.llvm.org/D118633