[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: rupprecht.
labath added a comment.

Adding @rupprecht, as he might be interested in following this.

I don't want to get too involved in this, but the first thought on my mind is "do you really want to do all this from within a signal handler?". The fact that we're running this code means that we're already in a bad state, and its pretty hard to say how far we will manage to get without triggering another crash. At the very least, I think we should print the crash dump directory as the first order of business, before we start running random callbacks.

There are various ways to solve that, like moving the dumping code to another process, or being very careful about what you run inside the crash handler. The one thing that they all have in common is that they are harder to design/implement that what you have now. So, if you want try this out, and accept the risk that it may not be enough to capture all the crashes you're interested in, then I won't stand in your way...



================
Comment at: lldb/include/lldb/Target/Statistics.h:180
 
+class Stats {
+public:
----------------
What's the difference between this class and DebuggerStats above? e.g. if I wanted to add some new method, how would I know where to add it?


================
Comment at: lldb/source/Utility/Diagnostics.cpp:54
+bool Diagnostics::Dump(raw_ostream &stream) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
----------------
I am not sure how common this is, but I have recently seen (not in lldb, but another app) a bug, which essentially caused two threads to crash at once (one SEGV, one ABRT). In those situations, you probably don't want to crash-printing routines to race with each other. You can consider putting a (global) mutex in this function, or something like that. (unless the llvm function takes care of that already).


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

https://reviews.llvm.org/D134991