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

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:439
+    // Record that an error occurred.
+    recordPath("");
     return false;
----------------
IIUC this slightly changes the semantics. Currently, even if we failed to download a file (for example because of the intermittent network error), we would try again on then next symbolization request. With this change, we would cache the error result (that is, an empty string) after the first attempt and never retry. Maybe it would be better to drop this line to preserve the current semantics?


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:368
+  SmallVector<uint8_t> BuildID = parseBuildIDArg(Args, OPT_build_id_EQ);
+  if (!BuildID.empty() && !Args.getLastArgValue(OPT_obj_EQ).empty()) {
+    errs() << "error: cannot specify both --build-id and --obj\n";
----------------
It might be more efficient to check if both `--obj` and `--build-id` was specified before parsing `--build-id` and allocating memory for build ID, but it only makes a difference for the error path so it's not particularly important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118633