[LLDB][ProcessELFCore] Add Description to ProcessELFCore/ELFThread stop reasons (#110065)

This fixes a functionality gap with GDB, where GDB will properly decode
the stop reason and give the address for SIGSEGV. I also added
descriptions to all stop reasons, following the same code path that the
Native Linux Thread uses.

Author: Jlalond

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

@@ -232,7 +232,7 @@ Status ProcessElfCore::DoLoadCore() {
   bool prstatus_signal_found = false;
   // Check we found a signal in a SIGINFO note.
   for (const auto &thread_data : m_thread_data) {
-    if (thread_data.signo != 0)
+    if (thread_data.siginfo.si_signo != 0)
       siginfo_signal_found = true;
     if (thread_data.prstatus_sig != 0)
       prstatus_signal_found = true;
@@ -242,10 +242,10 @@ Status ProcessElfCore::DoLoadCore() {
     // PRSTATUS note.
     if (prstatus_signal_found) {
       for (auto &thread_data : m_thread_data)
-        thread_data.signo = thread_data.prstatus_sig;
+        thread_data.siginfo.si_signo = thread_data.prstatus_sig;
     } else if (m_thread_data.size() > 0) {
       // If all else fails force the first thread to be SIGSTOP
-      m_thread_data.begin()->signo =
+      m_thread_data.begin()->siginfo.si_signo =
           GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
     }
   }
@@ -498,7 +498,7 @@ static void ParseFreeBSDPrStatus(ThreadData &thread_data,
   else
     offset += 16;
 
-  thread_data.signo = data.GetU32(&offset); // pr_cursig
+  thread_data.siginfo.si_signo = data.GetU32(&offset); // pr_cursig
   thread_data.tid = data.GetU32(&offset);   // pr_pid
   if (lp64)
     offset += 4;
@@ -581,7 +581,7 @@ static void ParseOpenBSDProcInfo(ThreadData &thread_data,
     return;
 
   offset += 4;
-  thread_data.signo = data.GetU32(&offset);
+  thread_data.siginfo.si_signo = data.GetU32(&offset);
 }
 
 llvm::Expected<std::vector<CoreNote>>
@@ -819,15 +819,15 @@ llvm::Error ProcessElfCore::parseNetBSDNotes(llvm::ArrayRef<CoreNote> notes) {
   // Signal targeted at the whole process.
   if (siglwp == 0) {
     for (auto &data : m_thread_data)
-      data.signo = signo;
+      data.siginfo.si_signo = signo;
   }
   // Signal destined for a particular LWP.
   else {
     bool passed = false;
 
     for (auto &data : m_thread_data) {
       if (data.tid == siglwp) {
-        data.signo = signo;
+        data.siginfo.si_signo = signo;
         passed = true;
         break;
       }
@@ -930,12 +930,12 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) {
       break;
     }
     case ELF::NT_SIGINFO: {
+      const lldb_private::UnixSignals &unix_signals = *GetUnixSignals();
       ELFLinuxSigInfo siginfo;
-      Status status = siginfo.Parse(note.data, arch);
+      Status status = siginfo.Parse(note.data, arch, unix_signals);
       if (status.Fail())
         return status.ToError();
-      thread_data.signo = siginfo.si_signo;
-      thread_data.code = siginfo.si_code;
+      thread_data.siginfo = siginfo;
       break;
     }
     case ELF::NT_FILE: {

lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

@@ -9,6 +9,7 @@
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/UnixSignals.h"
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -49,9 +50,9 @@ using namespace lldb_private;
 
 // Construct a Thread object with given data
 ThreadElfCore::ThreadElfCore(Process &process, const ThreadData &td)
-    : Thread(process, td.tid), m_thread_name(td.name), m_thread_reg_ctx_sp(),
-      m_signo(td.signo), m_code(td.code), m_gpregset_data(td.gpregset),
-      m_notes(td.notes) {}
+    : Thread(process, td.tid), m_thread_reg_ctx_sp(), m_thread_name(td.name),
+      m_gpregset_data(td.gpregset), m_notes(td.notes),
+      m_siginfo(std::move(td.siginfo)) {}
 
 ThreadElfCore::~ThreadElfCore() { DestroyThread(); }
 
@@ -246,8 +247,21 @@ bool ThreadElfCore::CalculateStopInfo() {
   if (!process_sp)
     return false;
 
+  lldb::UnixSignalsSP unix_signals_sp(process_sp->GetUnixSignals());
+  if (!unix_signals_sp)
+    return false;
+
+  const char *sig_description;
+  std::string description = m_siginfo.GetDescription(*unix_signals_sp);
+  if (description.empty())
+    sig_description = nullptr;
+  else
+    sig_description = description.c_str();
+
   SetStopInfo(StopInfo::CreateStopReasonWithSignal(
-      *this, m_signo, /*description=*/nullptr, m_code));
+      *this, m_siginfo.si_signo, sig_description, m_siginfo.si_code));
+
+  SetStopInfo(m_stop_info_sp);
   return true;
 }
 
@@ -547,21 +561,64 @@ size_t ELFLinuxSigInfo::GetSize(const lldb_private::ArchSpec &arch) {
   }
 }
 
-Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) {
+Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch,
+                              const lldb_private::UnixSignals &unix_signals) {
   Status error;
-  if (GetSize(arch) > data.GetByteSize()) {
+  uint64_t size = GetSize(arch);
+  if (size > data.GetByteSize()) {
     error = Status::FromErrorStringWithFormat(
         "NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64,
         GetSize(arch), data.GetByteSize());
     return error;
   }
 
+  // Set that we've parsed the siginfo from a SIGINFO note.
+  note_type = eNT_SIGINFO;
   // Parsing from a 32 bit ELF core file, and populating/reusing the structure
   // properly, because the struct is for the 64 bit version
   offset_t offset = 0;
   si_signo = data.GetU32(&offset);
   si_errno = data.GetU32(&offset);
   si_code = data.GetU32(&offset);
+  // 64b ELF have a 4 byte pad.
+  if (data.GetAddressByteSize() == 8)
+    offset += 4;
+  // Not every stop signal has a valid address, but that will get resolved in
+  // the unix_signals.GetSignalDescription() call below.
+  if (unix_signals.GetShouldStop(si_signo)) {
+    // Instead of memcpy we call all these individually as the extractor will
+    // handle endianness for us.
+    sigfault.si_addr = data.GetAddress(&offset);
+    sigfault.si_addr_lsb = data.GetU16(&offset);
+    if (data.GetByteSize() - offset >= sizeof(sigfault.bounds)) {
+      sigfault.bounds._addr_bnd._lower = data.GetAddress(&offset);
+      sigfault.bounds._addr_bnd._upper = data.GetAddress(&offset);
+      sigfault.bounds._pkey = data.GetU32(&offset);
+    } else {
+      // Set these to 0 so we don't use bogus data for the description.
+      sigfault.bounds._addr_bnd._lower = 0;
+      sigfault.bounds._addr_bnd._upper = 0;
+      sigfault.bounds._pkey = 0;
+    }
+  }
 
   return error;
 }
+
+std::string ELFLinuxSigInfo::GetDescription(
+    const lldb_private::UnixSignals &unix_signals) const {
+  if (unix_signals.GetShouldStop(si_signo) && note_type == eNT_SIGINFO) {
+    if (sigfault.bounds._addr_bnd._upper != 0)
+      return unix_signals.GetSignalDescription(
+          si_signo, si_code, sigfault.si_addr, sigfault.bounds._addr_bnd._lower,
+          sigfault.bounds._addr_bnd._upper);
+    else
+      return unix_signals.GetSignalDescription(si_signo, si_code,
+                                               sigfault.si_addr);
+  }
+
+  // This looks weird, but there is an existing pattern where we don't pass a
+  // description to keep up with that, we return empty here, and then the above
+  // function will set the description whether or not this is empty.
+  return std::string();
+}

lldb/source/Plugins/Process/elf-core/ThreadElfCore.h

@@ -35,6 +35,8 @@ class ProcessInstanceInfo;
 #undef si_signo
 #undef si_code
 #undef si_errno
+#undef si_addr
+#undef si_addr_lsb
 
 struct ELFLinuxPrStatus {
   int32_t si_signo;
@@ -76,14 +78,36 @@ static_assert(sizeof(ELFLinuxPrStatus) == 112,
               "sizeof ELFLinuxPrStatus is not correct!");
 
 struct ELFLinuxSigInfo {
-  int32_t si_signo;
-  int32_t si_code;
+
+  int32_t si_signo; // Order matters for the first 3.
   int32_t si_errno;
+  int32_t si_code;
+  // Copied from siginfo_t so we don't have to include signal.h on non 'Nix
+  // builds.
+  struct {
+    lldb::addr_t si_addr;  /* faulting insn/memory ref. */
+    short int si_addr_lsb; /* Valid LSB of the reported address.  */
+    union {
+      /* used when si_code=SEGV_BNDERR */
+      struct {
+        lldb::addr_t _lower;
+        lldb::addr_t _upper;
+      } _addr_bnd;
+      /* used when si_code=SEGV_PKUERR */
+      uint32_t _pkey;
+    } bounds;
+  } sigfault;
+
+  enum { eUnspecified, eNT_SIGINFO } note_type;
 
   ELFLinuxSigInfo();
 
   lldb_private::Status Parse(const lldb_private::DataExtractor &data,
-                             const lldb_private::ArchSpec &arch);
+                             const lldb_private::ArchSpec &arch,
+                             const lldb_private::UnixSignals &unix_signals);
+
+  std::string
+  GetDescription(const lldb_private::UnixSignals &unix_signals) const;
 
   // Return the bytesize of the structure
   // 64 bit - just sizeof
@@ -93,7 +117,7 @@ struct ELFLinuxSigInfo {
   static size_t GetSize(const lldb_private::ArchSpec &arch);
 };
 
-static_assert(sizeof(ELFLinuxSigInfo) == 12,
+static_assert(sizeof(ELFLinuxSigInfo) == 56,
               "sizeof ELFLinuxSigInfo is not correct!");
 
 // PRPSINFO structure's size differs based on architecture.
@@ -142,10 +166,9 @@ struct ThreadData {
   lldb_private::DataExtractor gpregset;
   std::vector<lldb_private::CoreNote> notes;
   lldb::tid_t tid;
-  int signo = 0;
-  int code = 0;
-  int prstatus_sig = 0;
   std::string name;
+  ELFLinuxSigInfo siginfo;
+  int prstatus_sig = 0;
 };
 
 class ThreadElfCore : public lldb_private::Thread {
@@ -176,16 +199,17 @@ class ThreadElfCore : public lldb_private::Thread {
       m_thread_name.clear();
   }
 
+  void CreateStopFromSigInfo(const ELFLinuxSigInfo &siginfo,
+                             const lldb_private::UnixSignals &unix_signals);
+
 protected:
   // Member variables.
   std::string m_thread_name;
   lldb::RegisterContextSP m_thread_reg_ctx_sp;
 
-  int m_signo;
-  int m_code;
-
   lldb_private::DataExtractor m_gpregset_data;
   std::vector<lldb_private::CoreNote> m_notes;
+  ELFLinuxSigInfo m_siginfo;
 
   bool CalculateStopInfo() override;
 };

lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py

@@ -2,7 +2,6 @@
 Test that memory tagging features work with Linux core files.
 """
 
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -216,8 +215,7 @@ def test_mte_tag_fault_reason(self):
         self.expect(
             "bt",
             substrs=[
-                "* thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV: "
-                "sync tag check fault"
+                "* thread #1, name = 'a.out.mte', stop reason = SIGSEGV: sync tag check fault (fault address: 0xffff82c74010)"
             ],
         )
 

lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py

@@ -6,7 +6,6 @@
 API level it won't if we don't remove them there also.
 """
 
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -199,7 +198,13 @@ def test_non_address_bit_memory_caching(self):
     def test_non_address_bit_memory_corefile(self):
         self.runCmd("target create --core corefile")
 
-        self.expect("thread list", substrs=["stopped", "stop reason = signal SIGSEGV"])
+        self.expect(
+            "thread list",
+            substrs=[
+                "stopped",
+                "stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)",
+            ],
+        )
 
         # No caching (the program/corefile are the cache) and no writing
         # to memory. So just check that tagged/untagged addresses read

lldb/test/Shell/Register/Core/x86-32-linux-multithread.test

@@ -1,7 +1,7 @@
 # RUN: %lldb -b -s %s -c %p/Inputs/x86-32-linux-multithread.core | FileCheck %s
 
 thread list
-# CHECK: * thread #1: tid = 330633, 0x080492d2, name = 'a.out', stop reason = signal SIGSEGV
+# CHECK: * thread #1: tid = 330633, 0x080492d2, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)
 # CHECK-NEXT:   thread #2: tid = 330634, 0x080492dd, stop reason = signal 0
 # CHECK-NEXT:   thread #3: tid = 330635, 0x080492dd, stop reason = signal 0
 # CHECK-NEXT:   thread #4: tid = 330632, 0xf7f59549, stop reason = signal 0

lldb/test/Shell/Register/Core/x86-64-linux-multithread.test

@@ -1,7 +1,7 @@
 # RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux-multithread.core | FileCheck %s
 
 thread list
-# CHECK: * thread #1: tid = 329384, 0x0000000000401262, name = 'a.out', stop reason = signal SIGSEGV
+# CHECK: * thread #1: tid = 329384, 0x0000000000401262, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)
 # CHECK-NEXT:   thread #2: tid = 329385, 0x000000000040126d, stop reason = signal 0
 # CHECK-NEXT:   thread #3: tid = 329386, 0x000000000040126d, stop reason = signal 0
 # CHECK-NEXT:   thread #4: tid = 329383, 0x00007fcf5582f762, stop reason = signal 0