[PATCH] D101340: Allows for dsymutil crashes to generate reproduceable information

[Fred Grim via Phabricator via llvm-commits]

feg208 marked 11 inline comments as done.
feg208 added a comment.

I rolled up all the comments save one where I just have an outstanding question and then I'll roll that up as well



================
Comment at: llvm/tools/dsymutil/Reproducer.cpp:39
+Reproducer::Reproducer(StringRef Rt)
+    : VFS{vfs::getRealFileSystem()}, Root{std::move(Rt)} {}
+
----------------
JDevlieghere wrote:
> LLVM generally does not prefer braced initializer lists (https://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor) 
Got it. Thanks for passing this along. I really should have reviewed it before this submission but I certainly will before the next


================
Comment at: llvm/tools/dsymutil/dsymutil.cpp:680-682
     }
-
     Threads.wait();
     if (!AllOK)
----------------
JDevlieghere wrote:
> Unrelated whitespace change?
yeah. Sorry it snuck in.


================
Comment at: llvm/tools/dsymutil/dsymutil.cpp:710-718
+  auto VFS = (*Repro)->getVFS();
+  for (auto &InputFile : Options.InputFiles) {
+    auto DebugMapPtrsOrErr =
+        parseDebugMap(VFS, InputFile, Options.Archs, {}, false, false, false);
+    if (auto EC = DebugMapPtrsOrErr.getError()) {
+      dbgs() << "cannot parse the debug map for '" << InputFile
+             << "': " << EC.message() << '\n';
----------------
JDevlieghere wrote:
> Why is this needed? 
So my understanding is that when dsymutil crashes we want the mapping.yaml file, the binary and associated .o file (assuming its compiled with -g). The latter maybe for some workflows? Reusing this gets us the object file but at the cost of parsing the debug map. So maybe I should just copy the binary in? I can do that or just parse enough to recreate the object files. Or maybe I am misunderstanding this entirely?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101340