DavidSpickett created this revision. Herald added subscribers: atanasyan, arichardson, sdardis, emaste. Herald added a project: All. DavidSpickett requested review of this revision. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits.
Previously we only looked at the si_signo field, so you got: (lldb) bt * thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV * frame #0: 0x00000000004007f4 This patch adds si_code so we can show: (lldb) bt * thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV: sync tag check fault * frame #0: 0x00000000004007f4 Core files (at least for Linux) don't contain the fault address, so that argument to GetCrashReasonString is now optional. (this is also the reason we can't do memory tag annotations) Removed the assert from GetCrashReason because both existing uses were already guarded and it let's me use it as a validity check. This is mainly used for multi threaded core files where the thread that stopped will have a valid signal number but the others will not. The order of errno and code was incorrect in ElfLinuxSigInfo::Parse. It was the order that a "swapped" siginfo arch would use, which for Linux, is only MIPS. We removed MIPS Linux support some time ago. See: https://github.com/torvalds/linux/blob/fe15c26ee26efa11741a7b632e9f23b01aca4cc6/include/uapi/asm-generic/siginfo.h#L121 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145377 Files: lldb/source/Plugins/Process/POSIX/CrashReason.cpp lldb/source/Plugins/Process/POSIX/CrashReason.h lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp lldb/source/Plugins/Process/elf-core/ThreadElfCore.h lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py llvm/docs/ReleaseNotes.rst
Index: llvm/docs/ReleaseNotes.rst =================================================================== --- llvm/docs/ReleaseNotes.rst +++ llvm/docs/ReleaseNotes.rst @@ -188,6 +188,10 @@ omit defaulted template parameters. The full template parameter list can still be viewed with `expr --raw-output`/`frame var --raw-output`. (`D141828 <https://reviews.llvm.org/D141828>`_) +* LLDB will now use the value of the ``siginfo`` ``si_code`` field to report + more specific signal types when debugging an ELF core file. For example + "sync tag check fault", which is a specific type of SIGSEGV. + Changes to Sanitizers --------------------- Index: lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py =================================================================== --- lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py +++ lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py @@ -166,3 +166,14 @@ # the MTE core file which does support it but does not allow writing tags. self.expect("memory tag write 0 1", substrs=["error: Process does not support memory tagging"], error=True) + + @skipIfLLVMTargetMissing("AArch64") + def test_mte_tag_fault_reason(self): + """ Test that we correctly report the fault reason. """ + self.runCmd("target create --core core.mte") + + # There is no fault address shown here because core files do not include + # si_addr. + self.expect("bt", substrs=[ + "* thread #1, name = 'a.out.mte', stop reason = signal SIGSEGV: " + "sync tag check fault"]) Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.h =================================================================== --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -128,6 +128,7 @@ std::vector<lldb_private::CoreNote> notes; lldb::tid_t tid; int signo = 0; + int code = 0; int prstatus_sig = 0; std::string name; }; @@ -166,6 +167,7 @@ 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; Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp =================================================================== --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -14,6 +14,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "Plugins/Process/POSIX/CrashReason.h" #include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h" #include "Plugins/Process/Utility/RegisterContextFreeBSD_mips64.h" #include "Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.h" @@ -39,6 +40,7 @@ #include "ThreadElfCore.h" #include <memory> +#include <optional> using namespace lldb; using namespace lldb_private; @@ -46,7 +48,8 @@ // 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_gpregset_data(td.gpregset), m_notes(td.notes) {} + m_signo(td.signo), m_code(td.code), m_gpregset_data(td.gpregset), + m_notes(td.notes) {} ThreadElfCore::~ThreadElfCore() { DestroyThread(); } @@ -221,11 +224,20 @@ bool ThreadElfCore::CalculateStopInfo() { ProcessSP process_sp(GetProcess()); - if (process_sp) { - SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, m_signo)); - return true; + if (!process_sp) + return false; + + siginfo_t info; + info.si_signo = m_signo; + info.si_code = m_code; + std::string description; + CrashReason reason = GetCrashReason(info); + if (reason != CrashReason::eInvalidCrashReason) { + description = GetCrashReasonString(reason, /*fault_addr=*/std::nullopt); } - return false; + SetStopInfo(StopInfo::CreateStopReasonWithSignal( + *this, m_signo, description.size() ? description.c_str() : nullptr)); + return true; } // Parse PRSTATUS from NOTE entry @@ -409,8 +421,8 @@ // properly, because the struct is for the 64 bit version offset_t offset = 0; si_signo = data.GetU32(&offset); - si_code = data.GetU32(&offset); si_errno = data.GetU32(&offset); + si_code = data.GetU32(&offset); return error; } Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp =================================================================== --- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -920,6 +920,7 @@ if (status.Fail()) return status.ToError(); thread_data.signo = siginfo.si_signo; + thread_data.code = siginfo.si_code; break; } case ELF::NT_FILE: { Index: lldb/source/Plugins/Process/POSIX/CrashReason.h =================================================================== --- lldb/source/Plugins/Process/POSIX/CrashReason.h +++ lldb/source/Plugins/Process/POSIX/CrashReason.h @@ -13,6 +13,7 @@ #include <csignal> +#include <optional> #include <string> enum class CrashReason { @@ -51,7 +52,10 @@ eFloatSubscriptRange }; -std::string GetCrashReasonString(CrashReason reason, lldb::addr_t fault_addr); +// fault_addr will be empty when used from a core file which does not record the +// fault address. +std::string GetCrashReasonString(CrashReason reason, + std::optional<lldb::addr_t> fault_addr); std::string GetCrashReasonString(CrashReason reason, const siginfo_t &info); const char *CrashReasonAsString(CrashReason reason); Index: lldb/source/Plugins/Process/POSIX/CrashReason.cpp =================================================================== --- lldb/source/Plugins/Process/POSIX/CrashReason.cpp +++ lldb/source/Plugins/Process/POSIX/CrashReason.cpp @@ -159,7 +159,8 @@ reinterpret_cast<uintptr_t>(info.si_addr)); } -std::string GetCrashReasonString(CrashReason reason, lldb::addr_t fault_addr) { +std::string GetCrashReasonString(CrashReason reason, + std::optional<lldb::addr_t> fault_addr) { std::string str; switch (reason) { @@ -169,11 +170,13 @@ case CrashReason::eInvalidAddress: str = "signal SIGSEGV: invalid address"; - AppendFaultAddr(str, fault_addr); + if (fault_addr) + AppendFaultAddr(str, *fault_addr); break; case CrashReason::ePrivilegedAddress: str = "signal SIGSEGV: address access protected"; - AppendFaultAddr(str, fault_addr); + if (fault_addr) + AppendFaultAddr(str, *fault_addr); break; case CrashReason::eBoundViolation: str = "signal SIGSEGV: bound violation"; @@ -183,7 +186,8 @@ break; case CrashReason::eSyncTagCheckFault: str = "signal SIGSEGV: sync tag check fault"; - AppendFaultAddr(str, fault_addr); + if (fault_addr) + AppendFaultAddr(str, *fault_addr); break; case CrashReason::eIllegalOpcode: str = "signal SIGILL: illegal instruction"; @@ -350,6 +354,5 @@ return GetCrashReasonForSIGILL(info); } - assert(false && "unexpected signal"); return CrashReason::eInvalidCrashReason; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits