[PATCH] D146391: Add DWARFContext generation from LinkGraphs, use in perf support

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

Thanks very much @pchintalapudi!

Is it possible to write a testcase for this?



================
Comment at: llvm/lib/ExecutionEngine/Orc/DebugInfoSupport.cpp:1
+#include "llvm/ExecutionEngine/Orc/DebugInfoSupport.h"
+
----------------
Needs LLVM licensing header.


================
Comment at: llvm/lib/ExecutionEngine/Orc/PerfSupportPlugin.cpp:1
 #include "llvm/ExecutionEngine/Orc/PerfSupportPlugin.h"
 
----------------
Needs LLVM licensing header.


================
Comment at: llvm/lib/ExecutionEngine/Orc/PerfSupportPlugin.cpp:151-153
   dbgs() << "Getting debug info for symbol " << Sym.getName() << " at address "
-         << Addr.getValue() << " with size " << Size << "\n";
+         << (void *)Addr.getValue() << " with size " << Size << "\n";
   dbgs() << "Section ordinal: " << Section.getOrdinal() << "\n";
----------------
I probably missed this in the previous review, but any debug lines should be wrapped in an `LLVM_DEBUG` macro prior to commit.


================
Comment at: llvm/lib/ExecutionEngine/Orc/PerfSupportPlugin.cpp:249
+    if (!EDC) {
+      // TODO fatal errors when debug info fails? Some other way of reporting?
+      consumeError(EDC.takeError());
----------------
This is a good question.

Right now errors can be sent to `ExecutionSession::reportError` and they won't automatically take down the session. If you give this error a discernible type ("BadDWARF"?) then clients could choose to ignore completely or log based on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146391