https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/110065
>From 0e31c0ad2b9db1ba9055bb4b984557100c0a9feb Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 24 Sep 2024 17:42:49 -0700 Subject: [PATCH 1/9] 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 0bc394ca58248b595a832dd1567b1389d3e07824 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:41:57 -0700 Subject: [PATCH 2/9] 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 8f6eab35a41fb91b75f9046cec8f7d0773be9001 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:42:38 -0700 Subject: [PATCH 3/9] 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 d2934db6bf180c4e7bc52f55992aabe069a18249 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 25 Sep 2024 17:49:32 -0700 Subject: [PATCH 4/9] 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 562b560ed8e2bc8c984c49c0bbdc92000f5ea3bc Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 10:01:46 -0700 Subject: [PATCH 5/9] 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 f173ea4f7a2cd3fbbb8621b74266063706806fd8 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 13:18:19 -0700 Subject: [PATCH 6/9] 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 b21b6dd1290a22b89997954e42ef292b4b7e7c13 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 15:37:10 -0700 Subject: [PATCH 7/9] 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 e98ac5330edf4b4bacf42e0b78efa174df47c1ff Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 3 Oct 2024 11:40:24 -0700 Subject: [PATCH 8/9] 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 397f094a0fa358b36c0b7effd6c9a892d3d88c2b Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 4 Oct 2024 09:48:45 -0700 Subject: [PATCH 9/9] 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); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits