[Lldb-commits] [lldb] dbc0cb0 - [trace] Avoid a crash in the dumper when disassembling fails

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-07-12T16:23:03-07:00
New Revision: dbc0cb0198129fbded7ff3be5d080cd12ad91783

URL: https://github.com/llvm/llvm-project/commit/dbc0cb0198129fbded7ff3be5d080cd12ad91783
DIFF: https://github.com/llvm/llvm-project/commit/dbc0cb0198129fbded7ff3be5d080cd12ad91783.diff

LOG: [trace] Avoid a crash in the dumper when disassembling fails

In rare situations, disassemblying would fail that produce an invalid
InstructionSP object. We need to check that it's valid before using.

With this change, now the dumper doesn't crash with dumping instructions of
ioctl. In fact, it now dumps this output

 {
    "id": 6135,
    "loadAddress": "0x7f4bfe5c7515",
    "module": "libc.so.6",
    "symbol": "ioctl",
    "source": "glibc/2.34/src/glibc-2.34/sysdeps/unix/syscall-template.S",
    "line": 120,
    "column": 0
 }

Anyway, we need to investigate why the diassembler failed disassembling that
instruction. From over 2B instructions I was disassembling today, just this
one failed, so this could be a bug in LLVM's core disassembler.

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

Added: 
    

Modified: 
    lldb/source/Target/TraceDumper.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/TraceDumper.cpp b/lldb/source/Target/TraceDumper.cpp
index cc5d7bfd5693..9e8b9d2b3df6 100644
--- a/lldb/source/Target/TraceDumper.cpp
+++ b/lldb/source/Target/TraceDumper.cpp
@@ -145,7 +145,7 @@ class OutputWriterCLI : public TraceDumper::OutputWriter {
       m_s << "(error) " << *item.error;
     } else {
       m_s.Format("{0:x+16}", item.load_address);
-      if (item.symbol_info) {
+      if (item.symbol_info && item.symbol_info->instruction) {
         m_s << "    ";
         item.symbol_info->instruction->Dump(
             &m_s, /*max_opcode_byte_size=*/0,
@@ -200,6 +200,35 @@ class OutputWriterJSON : public TraceDumper::OutputWriter {
 
   ~OutputWriterJSON() { m_j.arrayEnd(); }
 
+  void DumpEvent(const TraceDumper::TraceItem &item) {
+    m_j.attribute("event", TraceCursor::EventKindToString(*item.event));
+  }
+
+  void DumpInstruction(const TraceDumper::TraceItem &item) {
+    m_j.attribute("loadAddress", formatv("{0:x}", item.load_address));
+    if (item.symbol_info) {
+      m_j.attribute("module", ToOptionalString(GetModuleName(item)));
+      m_j.attribute(
+          "symbol",
+          ToOptionalString(item.symbol_info->sc.GetFunctionName().AsCString()));
+
+      if (item.symbol_info->instruction) {
+        m_j.attribute("mnemonic",
+                      ToOptionalString(item.symbol_info->instruction->GetMnemonic(
+                          &item.symbol_info->exe_ctx)));
+      }
+
+      if (IsLineEntryValid(item.symbol_info->sc.line_entry)) {
+        m_j.attribute(
+            "source",
+            ToOptionalString(
+                item.symbol_info->sc.line_entry.file.GetPath().c_str()));
+        m_j.attribute("line", item.symbol_info->sc.line_entry.line);
+        m_j.attribute("column", item.symbol_info->sc.line_entry.column);
+      }
+    }
+  }
+
   void TraceItem(const TraceDumper::TraceItem &item) override {
     m_j.object([&] {
       m_j.attribute("id", item.id);
@@ -209,9 +238,7 @@ class OutputWriterJSON : public TraceDumper::OutputWriter {
             item.tsc ? Optional<std::string>(std::to_string(*item.tsc)) : None);
 
       if (item.event) {
-        m_j.object([&] {
-          m_j.attribute("event", TraceCursor::EventKindToString(*item.event));
-        });
+        DumpEvent(item);
         return;
       }
 
@@ -221,26 +248,7 @@ class OutputWriterJSON : public TraceDumper::OutputWriter {
       }
 
       // we know we are seeing an actual instruction
-      m_j.attribute("loadAddress", formatv("{0:x}", item.load_address));
-      if (item.symbol_info) {
-        m_j.attribute("module", ToOptionalString(GetModuleName(item)));
-        m_j.attribute("symbol",
-                      ToOptionalString(
-                          item.symbol_info->sc.GetFunctionName().AsCString()));
-        m_j.attribute(
-            "mnemonic",
-            ToOptionalString(item.symbol_info->instruction->GetMnemonic(
-                &item.symbol_info->exe_ctx)));
-
-        if (IsLineEntryValid(item.symbol_info->sc.line_entry)) {
-          m_j.attribute(
-              "source",
-              ToOptionalString(
-                  item.symbol_info->sc.line_entry.file.GetPath().c_str()));
-          m_j.attribute("line", item.symbol_info->sc.line_entry.line);
-          m_j.attribute("column", item.symbol_info->sc.line_entry.column);
-        }
-      }
+      DumpInstruction(item);
     });
   }