[lldb/Plugins] Move SBTarget::GetExtendedCrashInformation to SBProcess

This patch moves the SB API method GetExtendedCrashInformation from
SBTarget to SBProcess since it only makes sense to call this method on a
sane process which might not be the case on a SBTarget object.

It also addresses some feedbacks received after landing the first patch
for the 'crash-info' feature.

Differential Revision: https://reviews.llvm.org/D75049

Signed-off-by: Med Ismail Bennani <[email protected]>

Author: medismailben

lldb/bindings/interface/SBProcess.i

@@ -346,6 +346,11 @@ public:
     bool
     GetDescription (lldb::SBStream &description);
 
+    %feature("autodoc", "
+    Returns the process' extended crash information.") GetExtendedCrashInformation;
+    lldb::SBStructuredData
+    GetExtendedCrashInformation ();
+
     uint32_t
     GetNumSupportedHardwareWatchpoints (lldb::SBError &error) const;
 

lldb/bindings/interface/SBTarget.i

@@ -949,12 +949,6 @@ public:
     void
     SetLaunchInfo (const lldb::SBLaunchInfo &launch_info);
 
-    %feature("autodoc", "
-    Returns the platform's process extended crash information.") GetExtendedCrashInformation;
-    lldb::SBStructuredData
-    GetExtendedCrashInformation ();
-
-
     void SetCollectingStats(bool v);
 
     bool GetCollectingStats();

lldb/include/lldb/API/SBProcess.h

@@ -222,6 +222,8 @@ class LLDB_API SBProcess {
 
   bool GetDescription(lldb::SBStream &description);
 
+  SBStructuredData GetExtendedCrashInformation();
+
   /// Start Tracing with the given SBTraceOptions.
   ///
   /// \param[in] options

lldb/include/lldb/API/SBStructuredData.h

@@ -91,6 +91,7 @@ class SBStructuredData {
   friend class SBTraceOptions;
   friend class SBDebugger;
   friend class SBTarget;
+  friend class SBProcess;
   friend class SBThread;
   friend class SBThreadPlan;
   friend class SBBreakpoint;

lldb/include/lldb/API/SBTarget.h

@@ -819,8 +819,6 @@ class LLDB_API SBTarget {
 
   void SetLaunchInfo(const lldb::SBLaunchInfo &launch_info);
 
-  SBStructuredData GetExtendedCrashInformation();
-
 protected:
   friend class SBAddress;
   friend class SBBlock;

lldb/include/lldb/Target/Platform.h

@@ -831,16 +831,16 @@ class Platform : public PluginInterface {
   /// nullptr. This dictionnary is generic and extensible, as it contains an
   /// array for each different type of crash information.
   ///
-  /// \param[in] target
-  ///     The target running the crashed process.
+  /// \param[in] process
+  ///     The crashed process.
   ///
   /// \return
   ///     A structured data dictionnary containing at each entry, the crash
   ///     information type as the entry key and the matching  an array as the
   ///     entry value. \b nullptr if not implemented or  if the process has no
   ///     crash information entry. \b error if an error occured.
   virtual llvm::Expected<StructuredData::DictionarySP>
-  FetchExtendedCrashInformation(lldb_private::Target &target) {
+  FetchExtendedCrashInformation(lldb_private::Process &process) {
     return nullptr;
   }
 

lldb/source/API/SBProcess.cpp

@@ -18,6 +18,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
@@ -1010,6 +1011,30 @@ bool SBProcess::GetDescription(SBStream &description) {
   return true;
 }
 
+SBStructuredData SBProcess::GetExtendedCrashInformation() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBStructuredData, SBProcess,
+                             GetExtendedCrashInformation);
+  SBStructuredData data;
+  ProcessSP process_sp(GetSP());
+  if (!process_sp)
+    return LLDB_RECORD_RESULT(data);
+
+  PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
+
+  if (!platform_sp)
+    return LLDB_RECORD_RESULT(data);
+
+  auto expected_data =
+      platform_sp->FetchExtendedCrashInformation(*process_sp.get());
+
+  if (!expected_data)
+    return LLDB_RECORD_RESULT(data);
+
+  StructuredData::ObjectSP fetched_data = *expected_data;
+  data.m_impl_up->SetObjectSP(fetched_data);
+  return LLDB_RECORD_RESULT(data);
+}
+
 uint32_t
 SBProcess::GetNumSupportedHardwareWatchpoints(lldb::SBError &sb_error) const {
   LLDB_RECORD_METHOD_CONST(uint32_t, SBProcess,
@@ -1385,6 +1410,8 @@ void RegisterMethods<SBProcess>(Registry &R) {
   LLDB_REGISTER_METHOD(lldb::addr_t, SBProcess, ReadPointerFromMemory,
                        (lldb::addr_t, lldb::SBError &));
   LLDB_REGISTER_METHOD(bool, SBProcess, GetDescription, (lldb::SBStream &));
+  LLDB_REGISTER_METHOD(lldb::SBStructuredData, SBProcess,
+                       GetExtendedCrashInformation, ());
   LLDB_REGISTER_METHOD_CONST(uint32_t, SBProcess,
                              GetNumSupportedHardwareWatchpoints,
                              (lldb::SBError &));

lldb/source/API/SBTarget.cpp

@@ -2388,30 +2388,6 @@ void SBTarget::SetLaunchInfo(const lldb::SBLaunchInfo &launch_info) {
     m_opaque_sp->SetProcessLaunchInfo(launch_info.ref());
 }
 
-SBStructuredData SBTarget::GetExtendedCrashInformation() {
-  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBStructuredData, SBTarget,
-                             GetExtendedCrashInformation);
-  SBStructuredData data;
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
-    return LLDB_RECORD_RESULT(data);
-
-  PlatformSP platform_sp = target_sp->GetPlatform();
-
-  if (!target_sp)
-    return LLDB_RECORD_RESULT(data);
-
-  auto expected_data =
-      platform_sp->FetchExtendedCrashInformation(*target_sp.get());
-
-  if (!expected_data)
-    return LLDB_RECORD_RESULT(data);
-
-  StructuredData::ObjectSP fetched_data = *expected_data;
-  data.m_impl_up->SetObjectSP(fetched_data);
-  return LLDB_RECORD_RESULT(data);
-}
-
 namespace lldb_private {
 namespace repro {
 
@@ -2654,8 +2630,6 @@ void RegisterMethods<SBTarget>(Registry &R) {
   LLDB_REGISTER_METHOD_CONST(lldb::SBLaunchInfo, SBTarget, GetLaunchInfo, ());
   LLDB_REGISTER_METHOD(void, SBTarget, SetLaunchInfo,
                        (const lldb::SBLaunchInfo &));
-  LLDB_REGISTER_METHOD(lldb::SBStructuredData, SBTarget,
-                       GetExtendedCrashInformation, ());
   LLDB_REGISTER_METHOD(
       size_t, SBTarget, ReadMemory,
       (const lldb::SBAddress, void *, size_t, lldb::SBError &));

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1283,7 +1283,7 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
       }
 
       auto expected_crash_info =
-          platform_sp->FetchExtendedCrashInformation(process->GetTarget());
+          platform_sp->FetchExtendedCrashInformation(*process);
 
       if (!expected_crash_info) {
         result.AppendError(llvm::toString(expected_crash_info.takeError()));

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

@@ -1503,10 +1503,10 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir) {
 }
 
 llvm::Expected<StructuredData::DictionarySP>
-PlatformDarwin::FetchExtendedCrashInformation(lldb_private::Target &target) {
+PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
 
-  StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(target);
+  StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(process);
 
   if (!annotations || !annotations->GetSize()) {
     LLDB_LOG(log, "Couldn't extract crash information annotations");
@@ -1522,11 +1522,11 @@ PlatformDarwin::FetchExtendedCrashInformation(lldb_private::Target &target) {
 }
 
 StructuredData::ArraySP
-PlatformDarwin::ExtractCrashInfoAnnotations(Target &target) {
+PlatformDarwin::ExtractCrashInfoAnnotations(Process &process) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   ConstString section_name("__crash_info");
-  ProcessSP process_sp = target.GetProcessSP();
+  Target &target = process.GetTarget();
   StructuredData::ArraySP array_sp = std::make_shared<StructuredData::Array>();
 
   for (ModuleSP module : target.GetImages().Modules()) {
@@ -1562,8 +1562,8 @@ PlatformDarwin::ExtractCrashInfoAnnotations(Target &target) {
     Status error;
     CrashInfoAnnotations annotations;
     size_t expected_size = sizeof(CrashInfoAnnotations);
-    size_t bytes_read = process_sp->ReadMemoryFromInferior(
-        load_addr, &annotations, expected_size, error);
+    size_t bytes_read = process.ReadMemoryFromInferior(load_addr, &annotations,
+                                                       expected_size, error);
 
     if (expected_size != bytes_read || error.Fail()) {
       LLDB_LOG(log, "Failed to read {0} section from memory in module {1}: {2}",
@@ -1587,7 +1587,7 @@ PlatformDarwin::ExtractCrashInfoAnnotations(Target &target) {
 
     std::string message;
     bytes_read =
-        process_sp->ReadCStringFromMemory(annotations.message, message, error);
+        process.ReadCStringFromMemory(annotations.message, message, error);
 
     if (message.empty() || bytes_read != message.size() || error.Fail()) {
       LLDB_LOG(log, "Failed to read the message from memory in module {0}: {1}",
@@ -1603,8 +1603,8 @@ PlatformDarwin::ExtractCrashInfoAnnotations(Target &target) {
       LLDB_LOG(log, "No message2 available for module {0}.", module_name);
 
     std::string message2;
-    bytes_read = process_sp->ReadCStringFromMemory(annotations.message2,
-                                                   message2, error);
+    bytes_read =
+        process.ReadCStringFromMemory(annotations.message2, message2, error);
 
     if (!message2.empty() && bytes_read == message2.size() && error.Success())
       if (message2.back() == '\n')

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

@@ -86,7 +86,7 @@ class PlatformDarwin : public PlatformPOSIX {
   };
 
   llvm::Expected<lldb_private::StructuredData::DictionarySP>
-  FetchExtendedCrashInformation(lldb_private::Target &target) override;
+  FetchExtendedCrashInformation(lldb_private::Process &process) override;
 
 protected:
   struct CrashInfoAnnotations {
@@ -107,15 +107,15 @@ class PlatformDarwin : public PlatformPOSIX {
   /// extract the section to gather the messages annotations and the abort
   /// cause.
   ///
-  /// \param[in] target
-  ///     The target running the crashed process.
+  /// \param[in] process
+  ///     The crashed process.
   ///
   /// \return
   ///     A  structured data array containing at each entry in each entry, the
   ///     module spec, its UUID, the crash messages and the abort cause.
   ///     \b nullptr if process has no crash information annotations.
   lldb_private::StructuredData::ArraySP
-  ExtractCrashInfoAnnotations(lldb_private::Target &target);
+  ExtractCrashInfoAnnotations(lldb_private::Process &process);
 
   void ReadLibdispatchOffsetsAddress(lldb_private::Process *process);
 

lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py

@@ -8,6 +8,8 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbtest
+
 
 class PlatformProcessCrashInfoTestCase(TestBase):
 
@@ -17,7 +19,7 @@ def setUp(self):
         TestBase.setUp(self)
         self.runCmd("settings set auto-confirm true")
         self.source = "main.c"
-        self.line = 3
+        self.line = line_number(self.source, '// break here')
 
     def tearDown(self):
         self.runCmd("settings clear auto-confirm")
@@ -52,7 +54,10 @@ def test_api(self):
         stream = lldb.SBStream()
         self.assertTrue(stream)
 
-        crash_info = target.GetExtendedCrashInformation()
+        process = target.GetProcess()
+        self.assertTrue(process)
+
+        crash_info = process.GetExtendedCrashInformation()
 
         error = crash_info.GetAsJSON(stream)
 
@@ -62,24 +67,6 @@ def test_api(self):
 
         self.assertIn("pointer being freed was not allocated", stream.GetData())
 
-    @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)
-
-        stream = lldb.SBStream()
-        self.assertTrue(stream)
-
-        crash_info = target.GetExtendedCrashInformation()
-
-        error = crash_info.GetAsJSON(stream)
-        self.assertFalse(error.Success())
-        self.assertIn("No structured data.", error.GetCString())
-
-    @skipUnlessDarwin
     def test_on_sane_process(self):
         """Test that lldb doesn't fetch the extended crash information
         dictionnary from a 'sane' stopped process."""
@@ -90,7 +77,10 @@ def test_on_sane_process(self):
         stream = lldb.SBStream()
         self.assertTrue(stream)
 
-        crash_info = target.GetExtendedCrashInformation()
+        process = target.GetProcess()
+        self.assertTrue(process)
+
+        crash_info = process.GetExtendedCrashInformation()
 
         error = crash_info.GetAsJSON(stream)
         self.assertFalse(error.Success())

lldb/test/API/functionalities/process_crash_info/main.c

@@ -1,6 +1,7 @@
 #include <stdlib.h>
+
 int main() {
-  int *var = malloc(sizeof(int));
+  int *var = malloc(sizeof(int)); // break here
   free(var);
   free(var);
   return 0;