[Lldb-commits] [PATCH] D74657: [lldb/Plugins] Add ability to fetch crash information on crashed processes

Mon Feb 24 01:09:50 PST 2020

labath added a comment.

A couple of more comments from me. The two on the test are minor, but I think it would be good to resolve the one about the SB method placement soon, before this thing starts having users.

================
Comment at: lldb/bindings/interface/SBTarget.i:952-957
+    %feature("autodoc", "
+    Returns the platform's process extended crash information.") GetExtendedCrashInformation;
+    lldb::SBStructuredData
+    GetExtendedCrashInformation ();
+
+
----------------
I know it was my idea to have this on the SBTarget class, but I think we should move this once more :/. I mean, this is definitely better than the SBPlatform class (which is what I was really trying to say before), but I think SBProcess would be even better here. The SBTarget methods are things that (more or less) make sense even when you don't have an actual process around. However, "crash info" (I think) only makes sense once you have an actual process, and so it would make sense to have it somewhere near SBProcess::GetState/GetDescription. It would also be more symmetric with the CLI interface for this, which lives under the "process" command tree.

The internal interfaces might also be better off passing a Process object (instead of Target) around, but that is not as important..

Sorry. :(

================
Comment at: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py:20
+        self.source = "main.c"
+        self.line = 3
+
----------------
We have a `line_number` utility function to avoid hardcoding line numbers like this.

================
Comment at: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py:65-82
+    @skipUnlessDarwin
+    def test_before_launch(self):
+        """Test that lldb doesn't fetch the extended crash information
+        dictionnary from if the process wasn't launched yet."""
+        self.build()
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+        self.assertTrue(target, VALID_TARGET)
----------------
I'm hoping that these two tests can actually run (and pass) on non-darwin systems too...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74657