https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/110065
>From ce87c9d2b62c9b05e8ef76d7cc4036420ee563f3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 24 Sep 2024 17:42:49 -0700 Subject: [PATCH 01/13] Add the addr info when appropriate for NIX' crash signals --- .../Process/elf-core/ProcessElfCore.cpp | 1 + .../Process/elf-core/ThreadElfCore.cpp | 44 ++++++++++++++++--- .../Plugins/Process/elf-core/ThreadElfCore.h | 18 ++++++-- .../postmortem/elf-core/TestLinuxCore.py | 17 +++++++ .../elf-core/linux-x86_64-sigsev.yaml | 8 ++++ 5 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 7955594bf5d94c..468a3b8934e741 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -931,6 +931,7 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) { return status.ToError(); thread_data.signo = siginfo.si_signo; thread_data.code = siginfo.si_code; + thread_data.description = siginfo.GetDescription(); break; } case ELF::NT_FILE: { diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index 52b96052bdbeca..3260caabd70ac6 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -6,9 +6,11 @@ // //===----------------------------------------------------------------------===// +#include "Plugins/Process/POSIX/CrashReason.h" #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" @@ -41,6 +43,7 @@ #include "RegisterContextPOSIXCore_x86_64.h" #include "ThreadElfCore.h" +#include "bits/types/siginfo_t.h" #include <memory> using namespace lldb; @@ -49,8 +52,8 @@ 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) {} + m_signo(td.signo), m_code(td.code), m_sig_description(td.description), + m_gpregset_data(td.gpregset), m_notes(td.notes) {} ThreadElfCore::~ThreadElfCore() { DestroyThread(); } @@ -241,7 +244,7 @@ bool ThreadElfCore::CalculateStopInfo() { return false; SetStopInfo(StopInfo::CreateStopReasonWithSignal( - *this, m_signo, /*description=*/nullptr, m_code)); + *this, m_signo, m_sig_description.c_str(), m_code)); return true; } @@ -543,7 +546,8 @@ size_t ELFLinuxSigInfo::GetSize(const lldb_private::ArchSpec &arch) { Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { 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()); @@ -556,6 +560,36 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { 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; + switch (si_signo) { + case SIGFPE: + case SIGILL: + case SIGSEGV: + case SIGBUS: + case SIGTRAP: + addr = (void*)data.GetAddress(&offset); + addr_lsb = data.GetU16(&offset); + return error; + default: + return error; + } +} - return error; +std::string ELFLinuxSigInfo::GetDescription() { + switch (si_signo) { + case SIGFPE: + case SIGILL: + case SIGSEGV: + case SIGBUS: + case SIGTRAP: + return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( + si_signo, si_code, + reinterpret_cast<uintptr_t>(addr)); + default: + return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( + si_signo, si_code + ); + } } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 3fa0b8b0eedb0b..0dc21a10ded5e5 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -75,16 +75,25 @@ struct ELFLinuxPrStatus { static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!"); +union ELFSigval { + int sival_int; + void *sival_ptr; +}; + 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; + void *addr; /* faulting insn/memory ref. */ + int32_t addr_lsb; /* Valid LSB of the reported address. */ ELFLinuxSigInfo(); lldb_private::Status Parse(const lldb_private::DataExtractor &data, const lldb_private::ArchSpec &arch); + std::string GetDescription(); + // Return the bytesize of the structure // 64 bit - just sizeof // 32 bit - hardcoded because we are reusing the struct, but some of the @@ -93,7 +102,7 @@ struct ELFLinuxSigInfo { static size_t GetSize(const lldb_private::ArchSpec &arch); }; -static_assert(sizeof(ELFLinuxSigInfo) == 12, +static_assert(sizeof(ELFLinuxSigInfo) == 32, "sizeof ELFLinuxSigInfo is not correct!"); // PRPSINFO structure's size differs based on architecture. @@ -144,7 +153,9 @@ struct ThreadData { lldb::tid_t tid; int signo = 0; int code = 0; + void* sigaddr = nullptr; int prstatus_sig = 0; + std::string description; std::string name; }; @@ -183,6 +194,7 @@ class ThreadElfCore : public lldb_private::Thread { int m_signo; int m_code; + std::string m_sig_description; lldb_private::DataExtractor m_gpregset_data; std::vector<lldb_private::CoreNote> m_notes; diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index 7e8531c88bf34c..1f0dee1e86b682 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -10,6 +10,7 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil +from fblldb import fbvscode class LinuxCoreTestCase(TestBase): @@ -833,6 +834,22 @@ def test_riscv64_regs_gpr_only(self): substrs=["registers were unavailable"], ) + def test_sigsev_stopreason(self): + """ + Test that the address is included in the stop reason for a SIGSEV + """ + src = self.getSourcePath("linux-x64-sigsev.yaml") + obj_file = self.getBuildArtifact("sigsev.out") + fbvscode.set_trace() + self.yaml2obj(src, obj_file) + target = self.dbg.CreateTarget(None) + process = target.LoadCore(obj_file) + self.assertTrue(process, PROCESS_IS_VALID) + stop_reason = process.GetThreadAtIndex(0).GetStopDescription(128) + self.assertEqual(process.GetNumThreads(), 1) + self.assertIn("SIGSEGV: address not mapped to object (fault address: 0x1000)", stop_reason) + + def test_get_core_file_api(self): """ Test SBProcess::GetCoreFile() API can successfully get the core file. diff --git a/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml b/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml new file mode 100644 index 00000000000000..2adf30a36918a2 --- /dev/null +++ b/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml @@ -0,0 +1,8 @@ +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + OSABI: ELFOSABI_FREEBSD + Type: ET_EXEC + Machine: EM_X86_64 + Entry: 0xFFFFFFFF8037C000 >From ebc02c4e425a8a497bade1e4079959e6ebf6eea6 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:41:57 -0700 Subject: [PATCH 02/13] Drop test as we can't yamilize the pt_note content --- .../postmortem/elf-core/TestLinuxCore.py | 17 ----------------- .../elf-core/linux-x86_64-sigsev.yaml | 8 -------- 2 files changed, 25 deletions(-) delete mode 100644 lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index 1f0dee1e86b682..7e8531c88bf34c 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -10,7 +10,6 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil -from fblldb import fbvscode class LinuxCoreTestCase(TestBase): @@ -834,22 +833,6 @@ def test_riscv64_regs_gpr_only(self): substrs=["registers were unavailable"], ) - def test_sigsev_stopreason(self): - """ - Test that the address is included in the stop reason for a SIGSEV - """ - src = self.getSourcePath("linux-x64-sigsev.yaml") - obj_file = self.getBuildArtifact("sigsev.out") - fbvscode.set_trace() - self.yaml2obj(src, obj_file) - target = self.dbg.CreateTarget(None) - process = target.LoadCore(obj_file) - self.assertTrue(process, PROCESS_IS_VALID) - stop_reason = process.GetThreadAtIndex(0).GetStopDescription(128) - self.assertEqual(process.GetNumThreads(), 1) - self.assertIn("SIGSEGV: address not mapped to object (fault address: 0x1000)", stop_reason) - - def test_get_core_file_api(self): """ Test SBProcess::GetCoreFile() API can successfully get the core file. diff --git a/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml b/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml deleted file mode 100644 index 2adf30a36918a2..00000000000000 --- a/lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml +++ /dev/null @@ -1,8 +0,0 @@ ---- !ELF -FileHeader: - Class: ELFCLASS64 - Data: ELFDATA2LSB - OSABI: ELFOSABI_FREEBSD - Type: ET_EXEC - Machine: EM_X86_64 - Entry: 0xFFFFFFFF8037C000 >From d17c4127ef5049a61d6ea33f5440ca50026f39e1 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:42:38 -0700 Subject: [PATCH 03/13] Run GCF --- .../Process/elf-core/ThreadElfCore.cpp | 44 +++++++++---------- .../Plugins/Process/elf-core/ThreadElfCore.h | 10 ++--- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index 3260caabd70ac6..e9ae5211c28a53 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -6,7 +6,6 @@ // //===----------------------------------------------------------------------===// -#include "Plugins/Process/POSIX/CrashReason.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/StopInfo.h" #include "lldb/Target/Target.h" @@ -43,7 +42,6 @@ #include "RegisterContextPOSIXCore_x86_64.h" #include "ThreadElfCore.h" -#include "bits/types/siginfo_t.h" #include <memory> using namespace lldb; @@ -564,32 +562,30 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { if (data.GetAddressByteSize() == 8) offset += 4; switch (si_signo) { - case SIGFPE: - case SIGILL: - case SIGSEGV: - case SIGBUS: - case SIGTRAP: - addr = (void*)data.GetAddress(&offset); - addr_lsb = data.GetU16(&offset); - return error; - default: - return error; + case SIGFPE: + case SIGILL: + case SIGSEGV: + case SIGBUS: + case SIGTRAP: + addr = (void *)data.GetAddress(&offset); + addr_lsb = data.GetU16(&offset); + return error; + default: + return error; } } std::string ELFLinuxSigInfo::GetDescription() { switch (si_signo) { - case SIGFPE: - case SIGILL: - case SIGSEGV: - case SIGBUS: - case SIGTRAP: - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( - si_signo, si_code, - reinterpret_cast<uintptr_t>(addr)); - default: - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( - si_signo, si_code - ); + case SIGFPE: + case SIGILL: + case SIGSEGV: + case SIGBUS: + case SIGTRAP: + return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( + si_signo, si_code, reinterpret_cast<uintptr_t>(addr)); + default: + return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( + si_signo, si_code); } } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 0dc21a10ded5e5..3c6c02f73efae8 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -75,16 +75,16 @@ struct ELFLinuxPrStatus { static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!"); -union ELFSigval { - int sival_int; - void *sival_ptr; +union ELFSigval { + int sival_int; + void *sival_ptr; }; struct ELFLinuxSigInfo { int32_t si_signo; // Order matters for the first 3. int32_t si_errno; int32_t si_code; - void *addr; /* faulting insn/memory ref. */ + void *addr; /* faulting insn/memory ref. */ int32_t addr_lsb; /* Valid LSB of the reported address. */ ELFLinuxSigInfo(); @@ -153,7 +153,7 @@ struct ThreadData { lldb::tid_t tid; int signo = 0; int code = 0; - void* sigaddr = nullptr; + void *sigaddr = nullptr; int prstatus_sig = 0; std::string description; std::string name; >From f9e5cee26d2916a88e1a5e2737c949390ec3bbd6 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:49:32 -0700 Subject: [PATCH 04/13] remove addr from threaddata --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 3c6c02f73efae8..f2e4ad955b3de9 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -153,7 +153,6 @@ struct ThreadData { lldb::tid_t tid; int signo = 0; int code = 0; - void *sigaddr = nullptr; int prstatus_sig = 0; std::string description; std::string name; >From c3691a863405af7d34497c5ab38e218268425ca9 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 10:01:46 -0700 Subject: [PATCH 05/13] Refactor to not depend ont the SIG enums, and use lldb::addr_t --- .../Process/elf-core/ThreadElfCore.cpp | 35 +++++++++---------- .../Plugins/Process/elf-core/ThreadElfCore.h | 7 +--- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index e9ae5211c28a53..8da4d84127eb11 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -542,6 +542,14 @@ size_t ELFLinuxSigInfo::GetSize(const lldb_private::ArchSpec &arch) { } } +static bool IsSignalWithAddrValue(int signo) { + // SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP + // We can't use the enum here because it may not be available on windows or + // other platforms. We should make an LLDB platform agnostic enum for this + // in the future. + return signo == 8 || signo == 4 || signo == 11 || signo == 7 || signo == 5; +} + Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { Status error; uint64_t size = GetSize(arch); @@ -561,31 +569,20 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { // 64b ELF have a 4 byte pad. if (data.GetAddressByteSize() == 8) offset += 4; - switch (si_signo) { - case SIGFPE: - case SIGILL: - case SIGSEGV: - case SIGBUS: - case SIGTRAP: - addr = (void *)data.GetAddress(&offset); + if (IsSignalWithAddrValue(si_signo)) { + addr = data.GetAddress(&offset); addr_lsb = data.GetU16(&offset); - return error; - default: - return error; } + + return error; } std::string ELFLinuxSigInfo::GetDescription() { - switch (si_signo) { - case SIGFPE: - case SIGILL: - case SIGSEGV: - case SIGBUS: - case SIGTRAP: + if (IsSignalWithAddrValue(si_signo)) return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( si_signo, si_code, reinterpret_cast<uintptr_t>(addr)); - default: - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( + + + return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( si_signo, si_code); - } } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index f2e4ad955b3de9..f14fc45e7463a6 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -75,16 +75,11 @@ struct ELFLinuxPrStatus { static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!"); -union ELFSigval { - int sival_int; - void *sival_ptr; -}; - struct ELFLinuxSigInfo { int32_t si_signo; // Order matters for the first 3. int32_t si_errno; int32_t si_code; - void *addr; /* faulting insn/memory ref. */ + lldb::addr_t addr; /* faulting insn/memory ref. */ int32_t addr_lsb; /* Valid LSB of the reported address. */ ELFLinuxSigInfo(); >From 2598427b07df5655ecfd0f8d7bd044f2aee7e0b6 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 13:18:19 -0700 Subject: [PATCH 06/13] Fix tests for the entire string and run GCF --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp | 5 ++--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h | 4 ++-- .../mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py | 3 +-- .../TestAArch64LinuxNonAddressBitMemoryAccess.py | 2 +- lldb/test/Shell/Register/Core/x86-32-linux-multithread.test | 2 +- lldb/test/Shell/Register/Core/x86-64-linux-multithread.test | 2 +- 6 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index 8da4d84127eb11..9e669c54db1e5d 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -573,7 +573,7 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { addr = data.GetAddress(&offset); addr_lsb = data.GetU16(&offset); } - + return error; } @@ -582,7 +582,6 @@ std::string ELFLinuxSigInfo::GetDescription() { return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( si_signo, si_code, reinterpret_cast<uintptr_t>(addr)); - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( - si_signo, si_code); + si_signo, si_code); } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index f14fc45e7463a6..3d7c93fd7f1a8a 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -79,8 +79,8 @@ struct ELFLinuxSigInfo { int32_t si_signo; // Order matters for the first 3. int32_t si_errno; int32_t si_code; - lldb::addr_t addr; /* faulting insn/memory ref. */ - int32_t addr_lsb; /* Valid LSB of the reported address. */ + lldb::addr_t addr; /* faulting insn/memory ref. */ + int32_t addr_lsb; /* Valid LSB of the reported address. */ ELFLinuxSigInfo(); diff --git a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py index 0667759a341b83..aaa467e7bbaf08 100644 --- a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py +++ b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py @@ -216,8 +216,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)" ], ) diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py index d86070c37f98a3..f420da7a2d2742 100644 --- a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py +++ b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py @@ -199,7 +199,7 @@ 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 diff --git a/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test b/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test index 62e5bb8ee32fd7..eb0cf8708263cb 100644 --- a/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test +++ b/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 diff --git a/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test b/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test index ab28901cae9f20..a94a4de1c8080b 100644 --- a/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test +++ b/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 >From b6b46ae0a2296650b5665915fe6fd09f87af1d7c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 15:37:10 -0700 Subject: [PATCH 07/13] Python formatting --- .../TestAArch64LinuxMTEMemoryTagCoreFile.py | 1 - .../TestAArch64LinuxNonAddressBitMemoryAccess.py | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py index aaa467e7bbaf08..779050edb054a4 100644 --- a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py +++ b/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 * diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py index f420da7a2d2742..668fca11903660 100644 --- a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py +++ b/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 = SIGSEGV: address not mapped to object (fault address: 0x0)"]) + 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 >From 2eec5fbcc3c9e46e14aea6dd64b69bab5c7f4839 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 3 Oct 2024 11:40:24 -0700 Subject: [PATCH 08/13] Use unix signals from process instead of reinventing the wheel --- .../Process/elf-core/ProcessElfCore.cpp | 5 ++-- .../Process/elf-core/ThreadElfCore.cpp | 25 ++++++++----------- .../Plugins/Process/elf-core/ThreadElfCore.h | 5 ++-- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 468a3b8934e741..547ed7f67643f6 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -925,13 +925,14 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) { break; } case ELF::NT_SIGINFO: { + lldb::UnixSignalsSP unix_signals_sp = GetUnixSignals(); ELFLinuxSigInfo siginfo; - Status status = siginfo.Parse(note.data, arch); + Status status = siginfo.Parse(note.data, arch, unix_signals_sp); if (status.Fail()) return status.ToError(); thread_data.signo = siginfo.si_signo; thread_data.code = siginfo.si_code; - thread_data.description = siginfo.GetDescription(); + thread_data.description = siginfo.GetDescription(unix_signals_sp); break; } case ELF::NT_FILE: { diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index 9e669c54db1e5d..fcc1182bdbf000 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -542,15 +542,8 @@ size_t ELFLinuxSigInfo::GetSize(const lldb_private::ArchSpec &arch) { } } -static bool IsSignalWithAddrValue(int signo) { - // SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP - // We can't use the enum here because it may not be available on windows or - // other platforms. We should make an LLDB platform agnostic enum for this - // in the future. - return signo == 8 || signo == 4 || signo == 11 || signo == 7 || signo == 5; -} - -Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { +Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, + const lldb::UnixSignalsSP unix_signals_sp) { Status error; uint64_t size = GetSize(arch); if (size > data.GetByteSize()) { @@ -569,7 +562,9 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { // 64b ELF have a 4 byte pad. if (data.GetAddressByteSize() == 8) offset += 4; - if (IsSignalWithAddrValue(si_signo)) { + // Not every stop signal has a valid address, but that will get resolved in + // the unix_signals_sp->GetSignalDescription() call below. + if (unix_signals_sp->GetShouldStop(si_signo)) { addr = data.GetAddress(&offset); addr_lsb = data.GetU16(&offset); } @@ -577,11 +572,11 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) { return error; } -std::string ELFLinuxSigInfo::GetDescription() { - if (IsSignalWithAddrValue(si_signo)) - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( +std::string +ELFLinuxSigInfo::GetDescription(const lldb::UnixSignalsSP unix_signals_sp) { + if (unix_signals_sp->GetShouldStop(si_signo)) + return unix_signals_sp->GetSignalDescription( si_signo, si_code, reinterpret_cast<uintptr_t>(addr)); - return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription( - si_signo, si_code); + return unix_signals_sp->GetSignalDescription(si_signo, si_code); } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 3d7c93fd7f1a8a..dbd96c8e85d0b4 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -85,9 +85,10 @@ struct ELFLinuxSigInfo { ELFLinuxSigInfo(); lldb_private::Status Parse(const lldb_private::DataExtractor &data, - const lldb_private::ArchSpec &arch); + const lldb_private::ArchSpec &arch, + const lldb::UnixSignalsSP unix_signals_sp); - std::string GetDescription(); + std::string GetDescription(const lldb::UnixSignalsSP unix_signals_sp); // Return the bytesize of the structure // 64 bit - just sizeof >From 4a225c9023c42eb7d55d6afa4d1eed307c1301af Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 4 Oct 2024 09:48:45 -0700 Subject: [PATCH 09/13] Drop reinterpret cast --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index fcc1182bdbf000..b26370c32d7a22 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -576,7 +576,7 @@ std::string ELFLinuxSigInfo::GetDescription(const lldb::UnixSignalsSP unix_signals_sp) { if (unix_signals_sp->GetShouldStop(si_signo)) return unix_signals_sp->GetSignalDescription( - si_signo, si_code, reinterpret_cast<uintptr_t>(addr)); + si_signo, si_code, addr); return unix_signals_sp->GetSignalDescription(si_signo, si_code); } >From 4cacf295d1897820b2bf3e2140362cb31de16de8 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 7 Oct 2024 10:13:35 -0700 Subject: [PATCH 10/13] Copy the siginfo_t structure with as minimal differences as possible, and rework parsing and description generating code --- .../Process/elf-core/ThreadElfCore.cpp | 29 +++++++++++++++---- .../Plugins/Process/elf-core/ThreadElfCore.h | 20 +++++++++++-- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index b26370c32d7a22..f950e6a8c0ccf8 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -565,8 +565,20 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, // Not every stop signal has a valid address, but that will get resolved in // the unix_signals_sp->GetSignalDescription() call below. if (unix_signals_sp->GetShouldStop(si_signo)) { - addr = data.GetAddress(&offset); - addr_lsb = data.GetU16(&offset); + // Instead of memcpy we call all these individually as the extractor will + // handle endianness for us. + _sigfault.sig_addr = data.GetAddress(&offset); + _sigfault.sig_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; @@ -574,9 +586,16 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, std::string ELFLinuxSigInfo::GetDescription(const lldb::UnixSignalsSP unix_signals_sp) { - if (unix_signals_sp->GetShouldStop(si_signo)) - return unix_signals_sp->GetSignalDescription( - si_signo, si_code, addr); + if (unix_signals_sp->GetShouldStop(si_signo)) { + if (_sigfault._bounds._addr_bnd._upper != 0) + return unix_signals_sp->GetSignalDescription( + si_signo, si_code, _sigfault.sig_addr, + _sigfault._bounds._addr_bnd._lower, + _sigfault._bounds._addr_bnd._upper); + else + return unix_signals_sp->GetSignalDescription(si_signo, si_code, + _sigfault.sig_addr); + } return unix_signals_sp->GetSignalDescription(si_signo, si_code); } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index dbd96c8e85d0b4..09286f7ff48348 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -79,8 +79,22 @@ struct ELFLinuxSigInfo { int32_t si_signo; // Order matters for the first 3. int32_t si_errno; int32_t si_code; - lldb::addr_t addr; /* faulting insn/memory ref. */ - int32_t addr_lsb; /* Valid LSB of the reported address. */ + // Copied from siginfo_t so we don't have to include signal.h on non 'Nix + // builds, we add `g` to the si_ prefix because siginfo_t defines them as + // macros. + struct { + lldb::addr_t sig_addr; /* faulting insn/memory ref. */ + short int sig_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; ELFLinuxSigInfo(); @@ -98,7 +112,7 @@ struct ELFLinuxSigInfo { static size_t GetSize(const lldb_private::ArchSpec &arch); }; -static_assert(sizeof(ELFLinuxSigInfo) == 32, +static_assert(sizeof(ELFLinuxSigInfo) == 48, "sizeof ELFLinuxSigInfo is not correct!"); // PRPSINFO structure's size differs based on architecture. >From 202b768f266d1a4e272231c6f2196f50d93a30f9 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 7 Oct 2024 10:27:36 -0700 Subject: [PATCH 11/13] Switch to const reference --- .../Plugins/Process/elf-core/ProcessElfCore.cpp | 6 +++--- .../Plugins/Process/elf-core/ThreadElfCore.cpp | 16 ++++++++-------- .../Plugins/Process/elf-core/ThreadElfCore.h | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 547ed7f67643f6..19d65f34076795 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -925,14 +925,14 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) { break; } case ELF::NT_SIGINFO: { - lldb::UnixSignalsSP unix_signals_sp = GetUnixSignals(); + const lldb_private::UnixSignals &unix_signals = *GetUnixSignals(); ELFLinuxSigInfo siginfo; - Status status = siginfo.Parse(note.data, arch, unix_signals_sp); + 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.description = siginfo.GetDescription(unix_signals_sp); + thread_data.description = siginfo.GetDescription(unix_signals); break; } case ELF::NT_FILE: { diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index f950e6a8c0ccf8..a04f1c817473c5 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -543,7 +543,7 @@ size_t ELFLinuxSigInfo::GetSize(const lldb_private::ArchSpec &arch) { } Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, - const lldb::UnixSignalsSP unix_signals_sp) { + const lldb_private::UnixSignals &unix_signals) { Status error; uint64_t size = GetSize(arch); if (size > data.GetByteSize()) { @@ -563,8 +563,8 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, if (data.GetAddressByteSize() == 8) offset += 4; // Not every stop signal has a valid address, but that will get resolved in - // the unix_signals_sp->GetSignalDescription() call below. - if (unix_signals_sp->GetShouldStop(si_signo)) { + // 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.sig_addr = data.GetAddress(&offset); @@ -585,17 +585,17 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, } std::string -ELFLinuxSigInfo::GetDescription(const lldb::UnixSignalsSP unix_signals_sp) { - if (unix_signals_sp->GetShouldStop(si_signo)) { +ELFLinuxSigInfo::GetDescription(const lldb_private::UnixSignals &unix_signals) { + if (unix_signals.GetShouldStop(si_signo)) { if (_sigfault._bounds._addr_bnd._upper != 0) - return unix_signals_sp->GetSignalDescription( + return unix_signals.GetSignalDescription( si_signo, si_code, _sigfault.sig_addr, _sigfault._bounds._addr_bnd._lower, _sigfault._bounds._addr_bnd._upper); else - return unix_signals_sp->GetSignalDescription(si_signo, si_code, + return unix_signals.GetSignalDescription(si_signo, si_code, _sigfault.sig_addr); } - return unix_signals_sp->GetSignalDescription(si_signo, si_code); + return unix_signals.GetSignalDescription(si_signo, si_code); } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 09286f7ff48348..630eb63539e20d 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -100,9 +100,9 @@ struct ELFLinuxSigInfo { lldb_private::Status Parse(const lldb_private::DataExtractor &data, const lldb_private::ArchSpec &arch, - const lldb::UnixSignalsSP unix_signals_sp); + const lldb_private::UnixSignals &unix_signals); - std::string GetDescription(const lldb::UnixSignalsSP unix_signals_sp); + std::string GetDescription(const lldb_private::UnixSignals &unix_signals); // Return the bytesize of the structure // 64 bit - just sizeof >From babd0e14fe9f69f05f0fa946b80847e9ca1f8472 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 8 Oct 2024 09:41:06 -0700 Subject: [PATCH 12/13] Start migrating away from threaddata containing the signal data --- .../Process/elf-core/ProcessElfCore.cpp | 18 ++++--- .../Process/elf-core/ThreadElfCore.cpp | 48 +++++++++++-------- .../Plugins/Process/elf-core/ThreadElfCore.h | 29 ++++++----- 3 files changed, 50 insertions(+), 45 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 19d65f34076795..c4da111163cce9 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/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"); } } @@ -493,7 +493,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; @@ -576,7 +576,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>> @@ -814,7 +814,7 @@ 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 { @@ -822,7 +822,7 @@ llvm::Error ProcessElfCore::parseNetBSDNotes(llvm::ArrayRef<CoreNote> notes) { for (auto &data : m_thread_data) { if (data.tid == siglwp) { - data.signo = signo; + data.siginfo.si_signo = signo; passed = true; break; } @@ -930,9 +930,7 @@ llvm::Error ProcessElfCore::parseLinuxNotes(llvm::ArrayRef<CoreNote> notes) { 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.description = siginfo.GetDescription(unix_signals); + thread_data.siginfo = siginfo; break; } case ELF::NT_FILE: { diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index a04f1c817473c5..b758f498014730 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -49,9 +49,8 @@ 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_sig_description(td.description), - m_gpregset_data(td.gpregset), m_notes(td.notes) {} + : Thread(process, td.tid), m_thread_reg_ctx_sp(), + m_gpregset_data(td.gpregset), m_notes(td.notes), m_siginfo(std::move(td.siginfo)) {} ThreadElfCore::~ThreadElfCore() { DestroyThread(); } @@ -59,6 +58,8 @@ void ThreadElfCore::RefreshStateAfterStop() { GetRegisterContext()->InvalidateIfNeeded(false); } + + RegisterContextSP ThreadElfCore::GetRegisterContext() { if (!m_reg_context_sp) { m_reg_context_sp = CreateRegisterContextForFrame(nullptr); @@ -241,8 +242,15 @@ bool ThreadElfCore::CalculateStopInfo() { if (!process_sp) return false; + lldb::UnixSignalsSP unix_signals_sp(process_sp->GetUnixSignals()); + if (!unix_signals_sp) + return false; + SetStopInfo(StopInfo::CreateStopReasonWithSignal( - *this, m_signo, m_sig_description.c_str(), m_code)); + *this, m_siginfo.si_signo, + m_siginfo.GetDescription(*unix_signals_sp).c_str(), m_siginfo.si_code)); + + SetStopInfo(m_stop_info_sp); return true; } @@ -567,17 +575,17 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, if (unix_signals.GetShouldStop(si_signo)) { // Instead of memcpy we call all these individually as the extractor will // handle endianness for us. - _sigfault.sig_addr = data.GetAddress(&offset); - _sigfault.sig_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); + 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; + sigfault.bounds._addr_bnd._lower = 0; + sigfault.bounds._addr_bnd._upper = 0; + sigfault.bounds._pkey = 0; } } @@ -585,16 +593,16 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, } std::string -ELFLinuxSigInfo::GetDescription(const lldb_private::UnixSignals &unix_signals) { - if (unix_signals.GetShouldStop(si_signo)) { - if (_sigfault._bounds._addr_bnd._upper != 0) +ELFLinuxSigInfo::GetDescription(const lldb_private::UnixSignals &unix_signals) const { + if (unix_signals.GetShouldStop(si_signo) && sigfault.si_addr != 0) { + if (sigfault.bounds._addr_bnd._upper != 0) return unix_signals.GetSignalDescription( - si_signo, si_code, _sigfault.sig_addr, - _sigfault._bounds._addr_bnd._lower, - _sigfault._bounds._addr_bnd._upper); + 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.sig_addr); + sigfault.si_addr); } return unix_signals.GetSignalDescription(si_signo, si_code); diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 630eb63539e20d..ff317207fab55c 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/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,15 +78,15 @@ static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!"); struct ELFLinuxSigInfo { + 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, we add `g` to the si_ prefix because siginfo_t defines them as - // macros. + // builds. struct { - lldb::addr_t sig_addr; /* faulting insn/memory ref. */ - short int sig_addr_lsb; /* Valid LSB of the reported address. */ + 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 { @@ -93,8 +95,8 @@ struct ELFLinuxSigInfo { } _addr_bnd; /* used when si_code=SEGV_PKUERR */ uint32_t _pkey; - } _bounds; - } _sigfault; + } bounds; + } sigfault; ELFLinuxSigInfo(); @@ -102,7 +104,7 @@ struct ELFLinuxSigInfo { const lldb_private::ArchSpec &arch, const lldb_private::UnixSignals &unix_signals); - std::string GetDescription(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 @@ -161,11 +163,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 description; std::string name; + ELFLinuxSigInfo siginfo; + int prstatus_sig = 0; }; class ThreadElfCore : public lldb_private::Thread { @@ -196,17 +196,16 @@ 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; - std::string m_sig_description; - lldb_private::DataExtractor m_gpregset_data; std::vector<lldb_private::CoreNote> m_notes; + ELFLinuxSigInfo m_siginfo; bool CalculateStopInfo() override; }; >From a4b7b3f4d1902ab5e2f758c30712ccd5520cfb60 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 8 Oct 2024 15:01:05 -0700 Subject: [PATCH 13/13] Move all the ELF Nix' flavors to the SigInfo, but don't emit unless we read 0 from NT_SIGINFO --- .../Process/elf-core/ThreadElfCore.cpp | 35 ++++++++++++------- .../Plugins/Process/elf-core/ThreadElfCore.h | 10 ++++-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index b758f498014730..10bdf03a1061b6 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -49,8 +49,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_reg_ctx_sp(), - m_gpregset_data(td.gpregset), m_notes(td.notes), m_siginfo(std::move(td.siginfo)) {} + : 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(); } @@ -58,8 +59,6 @@ void ThreadElfCore::RefreshStateAfterStop() { GetRegisterContext()->InvalidateIfNeeded(false); } - - RegisterContextSP ThreadElfCore::GetRegisterContext() { if (!m_reg_context_sp) { m_reg_context_sp = CreateRegisterContextForFrame(nullptr); @@ -246,9 +245,15 @@ bool ThreadElfCore::CalculateStopInfo() { 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_siginfo.si_signo, - m_siginfo.GetDescription(*unix_signals_sp).c_str(), m_siginfo.si_code)); + *this, m_siginfo.si_signo, sig_description, m_siginfo.si_code)); SetStopInfo(m_stop_info_sp); return true; @@ -561,6 +566,8 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, 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; @@ -592,18 +599,20 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch, return error; } -std::string -ELFLinuxSigInfo::GetDescription(const lldb_private::UnixSignals &unix_signals) const { - if (unix_signals.GetShouldStop(si_signo) && sigfault.si_addr != 0) { +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, + 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); + sigfault.si_addr); } - return unix_signals.GetSignalDescription(si_signo, si_code); + // 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(); } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index ff317207fab55c..4ebbaadebe9f90 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -98,13 +98,16 @@ struct ELFLinuxSigInfo { } 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::UnixSignals &unix_signals); - std::string GetDescription(const lldb_private::UnixSignals &unix_signals) const; + std::string + GetDescription(const lldb_private::UnixSignals &unix_signals) const; // Return the bytesize of the structure // 64 bit - just sizeof @@ -114,7 +117,7 @@ struct ELFLinuxSigInfo { static size_t GetSize(const lldb_private::ArchSpec &arch); }; -static_assert(sizeof(ELFLinuxSigInfo) == 48, +static_assert(sizeof(ELFLinuxSigInfo) == 56, "sizeof ELFLinuxSigInfo is not correct!"); // PRPSINFO structure's size differs based on architecture. @@ -196,7 +199,8 @@ class ThreadElfCore : public lldb_private::Thread { m_thread_name.clear(); } - void CreateStopFromSigInfo(const ELFLinuxSigInfo &siginfo, const lldb_private::UnixSignals &unix_signals); + void CreateStopFromSigInfo(const ELFLinuxSigInfo &siginfo, + const lldb_private::UnixSignals &unix_signals); protected: // Member variables. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits