[Lldb-commits] [PATCH] D126260: [lldb/crashlog] Add support for Application Specific Backtraces & Information

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

A few nits but overall this LGTM.



================
Comment at: lldb/examples/python/crashlog.py:601-605
+        for idx, backtrace in enumerate(json_app_specific_bts):
+            thread = self.crashlog.Thread(idx, True)
+            thread.queue = "Application Specific Backtrace"
+            if parse_asi_backtrace(self, thread, backtrace):
+                self.crashlog.threads.append(thread)
----------------
I don't think it's correct to treat the ASI as just another thread. IIUC it's the same as the crashing thread, just at another point in time (when the exception was thrown). To me that sounds like it should be its own thing.


================
Comment at: lldb/examples/python/crashlog.py:581-597
+        def parse_asi_backtrace(self, thread, bt):
+            for line in bt.split('\n'):
+                frame_match = TextCrashLogParser.frame_regex.search(line)
+                if not frame_match:
+                    print("error: can't parse application specific backtrace.")
+                    return False
+
----------------
mib wrote:
> JDevlieghere wrote:
> > Can this be a top level function? It's hard to tell if this is capturing anything. We have other place where we have helper functions that are called from a loop.
> This parses a single backtrace. I'm not sure what you mean by "We have other place where we have helper functions that are called from a loop."
ping


================
Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:59
+
+        if (self.app_specific_thread):
+            self.extended_thread_info = \
----------------



================
Comment at: lldb/include/lldb/Interpreter/ScriptedProcessInterface.h:105
+
+  virtual StructuredData::ArraySP GetExtendedInfo() { return nullptr; }
 };
----------------



================
Comment at: lldb/include/lldb/Target/Process.h:2431
+  ///     information related to the process.
+  virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+
----------------



================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:985
+  if (!metadata_sp || !metadata_sp->GetSize() || !metadata_sp->HasKey("asi"))
+    return nullptr;
+
----------------



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

https://reviews.llvm.org/D126260