llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Bar Soloveychik (barsolo2000) <details> <summary>Changes</summary> We encountered a situation where loading a core file created two modules that both define `__jit_debug_descriptor`, One was a stale on-disk copy of a binary and the other was the build-ID-matched copy of the same binary. They were mapped so that one of them resolved onto unrelated memory. `JITLoaderGDB::SetJITBreakpoint` looks up `__jit_debug_descriptor` and always takes the first match. Here that was the bad copy, whose first_entry read back as a non-null but misaligned garbage value. Afterwards `ReadJITEntry` was called which resulted in: ``` lldbassert(from_addr % sizeof(ptr_t) == 0); ``` `from_addr` comes from untrusted memory, so a misaligned value is bad input, not a reason to assert. This patch makes the JIT loader more tolerant: * `ReadJITEntry` returns false when handed a misaligned entry pointer, instead of asserting. * `SetJITBreakpoint` now picks a descriptor that reads back as plausible (its first_entry is null or aligned) when more than one module defines `__jit_debug_descriptor` (Instead of always taking the first one). We still fallback on first match when none validate, so single descriptor behavior is unchanged. test will show unresolved without the fix: [gist:d67365551157cec903896cfffe431fae](https://gist.github.com/barsolo2000/d67365551157cec903896cfffe431fae) --- Full diff: https://github.com/llvm/llvm-project/pull/201912.diff 4 Files Affected: - (modified) lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp (+64-3) - (modified) lldb/test/API/functionalities/jitloader_gdb/Makefile (+5-1) - (modified) lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py (+22) - (added) lldb/test/API/functionalities/jitloader_gdb/misaligned_first_entry.c (+32) ``````````diff diff --git a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp index b336a605b747a..e405d0fdbbcf7 100644 --- a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp +++ b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp @@ -115,7 +115,13 @@ static PluginProperties &GetGlobalPluginProperties() { template <typename ptr_t> static bool ReadJITEntry(const addr_t from_addr, Process *process, jit_code_entry<ptr_t> *entry) { - lldbassert(from_addr % sizeof(ptr_t) == 0); + // Skip a misaligned entry pointer (corrupt input) instead of asserting. + if (from_addr % sizeof(ptr_t) != 0) { + LLDB_LOGF(GetLog(LLDBLog::JITLoader), + "JITLoaderGDB::%s skipping misaligned JIT entry 0x%" PRIx64, + __FUNCTION__, from_addr); + return false; + } ArchSpec::Core core = process->GetTarget().GetArchitecture().GetCore(); bool i386_target = ArchSpec::kCore_x86_32_first <= core && @@ -143,6 +149,62 @@ static bool ReadJITEntry(const addr_t from_addr, Process *process, return true; } +// True if the descriptor reads back sane (first_entry is null or aligned). +template <typename ptr_t> +static bool JITDescriptorIsValid(addr_t descriptor_addr, Process *process) { + jit_descriptor<ptr_t> jit_desc; + Status error; + // Reject if the descriptor can't be fully read (unmapped/garbage address). + const size_t bytes_read = + process->ReadMemory(descriptor_addr, &jit_desc, sizeof(jit_desc), error); + if (bytes_read != sizeof(jit_desc) || !error.Success()) + return false; + // Valid only if first_entry (the chain head we walk) is null or aligned. + const addr_t first_entry = (addr_t)jit_desc.first_entry; + return first_entry == 0 || first_entry % sizeof(ptr_t) == 0; +} + +// Pick the first sane __jit_debug_descriptor among modules (fallback: first +// match). +static addr_t GetValidJITDescriptorAddress(ModuleList &module_list, + Process *process) { + // all matches + SymbolContextList target_symbols; + module_list.FindSymbolsWithNameAndType(ConstString("__jit_debug_descriptor"), + eSymbolTypeData, target_symbols); + if (target_symbols.IsEmpty()) + return LLDB_INVALID_ADDRESS; + + Target &target = process->GetTarget(); + const bool is_64bit = target.GetArchitecture().GetAddressByteSize() == 8; + + addr_t first_addr = LLDB_INVALID_ADDRESS; + // scan all the candidates. + for (uint32_t i = 0; i < target_symbols.GetSize(); ++i) { + SymbolContext sym_ctx; + target_symbols.GetContextAtIndex(i, sym_ctx); + if (!sym_ctx.symbol) + continue; + const Address descriptor_addr = sym_ctx.symbol->GetAddress(); + if (!descriptor_addr.IsValid()) + continue; + const addr_t load_addr = descriptor_addr.GetLoadAddress(&target); + if (load_addr == LLDB_INVALID_ADDRESS) + continue; + + if (first_addr == LLDB_INVALID_ADDRESS) + first_addr = load_addr; // fallback. + + // take first valid. + const bool valid = is_64bit + ? JITDescriptorIsValid<uint64_t>(load_addr, process) + : JITDescriptorIsValid<uint32_t>(load_addr, process); + if (valid) + return load_addr; + } + return first_addr; +} + JITLoaderGDB::JITLoaderGDB(lldb_private::Process *process) : JITLoader(process), m_jit_objects(), m_jit_break_id(LLDB_INVALID_BREAK_ID), @@ -194,8 +256,7 @@ void JITLoaderGDB::SetJITBreakpoint(lldb_private::ModuleList &module_list) { if (jit_addr == LLDB_INVALID_ADDRESS) return; - m_jit_descriptor_addr = GetSymbolAddress( - module_list, ConstString("__jit_debug_descriptor"), eSymbolTypeData); + m_jit_descriptor_addr = GetValidJITDescriptorAddress(module_list, m_process); if (m_jit_descriptor_addr == LLDB_INVALID_ADDRESS) { LLDB_LOGF(log, "JITLoaderGDB::%s failed to find JIT descriptor address", __FUNCTION__); diff --git a/lldb/test/API/functionalities/jitloader_gdb/Makefile b/lldb/test/API/functionalities/jitloader_gdb/Makefile index 9998cc9cf833c..5903c05c8ad3b 100644 --- a/lldb/test/API/functionalities/jitloader_gdb/Makefile +++ b/lldb/test/API/functionalities/jitloader_gdb/Makefile @@ -1,9 +1,13 @@ C_SOURCES := main.c -all: a.out simple +all: a.out simple misaligned include Makefile.rules simple: "$(MAKE)" -f $(MAKEFILE_RULES) \ C_SOURCES=simple.c EXE=simple + +misaligned: + "$(MAKE)" -f $(MAKEFILE_RULES) \ + C_SOURCES=misaligned_first_entry.c EXE=misaligned diff --git a/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py b/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py index 98c0b149003df..09a0f08ffc94a 100644 --- a/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py +++ b/lldb/test/API/functionalities/jitloader_gdb/TestJITLoaderGDB.py @@ -33,6 +33,28 @@ def test_bogus_values(self): self.assertState(process.GetState(), lldb.eStateExited) self.assertEqual(process.GetExitStatus(), 0) + @skipIfWindows # This test fails on Windows during C code build + def test_misaligned_first_entry(self): + """A single module whose descriptor has a misaligned first_entry must + not crash lldb. ReadJITEntry has to bail gracefully instead of + asserting.""" + self.build() + exe = self.getBuildArtifact("misaligned") + + self.runCmd("settings set plugin.jit-loader.gdb.enable on") + self.addTearDownHook( + lambda: self.runCmd("settings set plugin.jit-loader.gdb.enable default") + ) + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + # Without the fix this aborts in ReadJITEntry. + self.assertState(process.GetState(), lldb.eStateExited) + self.assertEqual(process.GetExitStatus(), 0) + def gen_log_file(self): logfile = self.getBuildArtifact( "jitintgdb-{}.txt".format(self.getArchitecture()) diff --git a/lldb/test/API/functionalities/jitloader_gdb/misaligned_first_entry.c b/lldb/test/API/functionalities/jitloader_gdb/misaligned_first_entry.c new file mode 100644 index 0000000000000..1a5ed928aeebc --- /dev/null +++ b/lldb/test/API/functionalities/jitloader_gdb/misaligned_first_entry.c @@ -0,0 +1,32 @@ +#include <inttypes.h> + +// GDB JIT interface +enum JITAction { JIT_NOACTION, JIT_REGISTER_FN, JIT_UNREGISTER_FN }; + +struct JITCodeEntry { + struct JITCodeEntry *next; + struct JITCodeEntry *prev; + const char *symfile_addr; + uint64_t symfile_size; +}; + +struct JITDescriptor { + uint32_t version; + uint32_t action_flag; + struct JITCodeEntry *relevant_entry; + struct JITCodeEntry *first_entry; +}; + +// A single module whose first_entry is non-null but *misaligned*, i.e. it can +// never be a real jit_code_entry pointer. lldb must not assert when it walks +// the chain (see JITLoaderGDB.cpp, ReadJITEntry). +#define BAD ((struct JITCodeEntry *)0x0300cee3fb4002ffULL) +struct JITDescriptor __jit_debug_descriptor = {1, JIT_REGISTER_FN, BAD, BAD}; + +void __jit_debug_register_code() {} +// end GDB JIT interface + +int main() { + __jit_debug_register_code(); + return 0; +} `````````` </details> https://github.com/llvm/llvm-project/pull/201912 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
