From ff50cf25522f06a9139089bc1b6a83d09ebcd613 Mon Sep 17 00:00:00 2001 From: ManDude <7569514+ManDude@users.noreply.github.com> Date: Sat, 13 Nov 2021 03:00:41 +0000 Subject: [PATCH] improve debugger disasm, `:sym-name` and fix Windows builds (#959) * improve debugger disasm, `:sym-name` and fix Windows builds * >:( * use this inline constexpr thing?? * fine use strings then * please.... please work... * fix windows debugger oopsie * display rip as goal addr as well * [debugger] attempt to backtrace even if landed on some garbage memory * Update CMakePresets.json --- CMakeLists.txt | 7 +- CMakePresets.json | 4 +- common/cross_os_debug/xdbg.cpp | 2 +- common/util/FileUtil.cpp | 2 + docs/markdown/debugging.md | 35 +++++----- game/kernel/kprint.cpp | 14 ++-- goalc/compiler/compilation/Debug.cpp | 16 ++--- goalc/debugger/Debugger.cpp | 97 +++++++++++++++++++++++++--- goalc/debugger/Debugger.h | 4 +- 9 files changed, 133 insertions(+), 48 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e2151a726..98fe7aa73a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,10 +31,9 @@ if(UNIX) -fdiagnostics-color=always") else() if(CMAKE_BUILD_TYPE MATCHES "Debug") - message("Setting Flags to Enable Edit and Continue") - set(CMAKE_CXX_FLAGS_DEBUG "/ZI") - set(CMAKE_SHARED_LINKER_FLAGS "/SAFESEH:NO") - set(CMAKE_EXE_LINKER_FLAGS "/SAFESEH:NO") + # This actually breaks some standard library things for some reason? + # message("Setting Flags to Enable Edit and Continue") + # set(CMAKE_CXX_FLAGS_DEBUG "/ZI") endif() set(CMAKE_CXX_FLAGS "/EHsc") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:16000000,16384") diff --git a/CMakePresets.json b/CMakePresets.json index 11089b59a9..10b916f18b 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -17,7 +17,7 @@ } }, { - "name": "windows-debug", + "name": "Debug", "displayName": "Windows x64 Debug", "description": "Target Windows with the Visual Studio development environment.", "generator": "Ninja", @@ -34,7 +34,7 @@ "vendor": { "microsoft.com/VisualStudioSettings/CMake/1.0": { "hostOS": [ "Windows" ] } } }, { - "name": "windows-release", + "name": "Release", "displayName": "Windows x64 Release", "description": "Target Windows with the Visual Studio development environment.", "generator": "Ninja", diff --git a/common/cross_os_debug/xdbg.cpp b/common/cross_os_debug/xdbg.cpp index 13dd14259c..24f460085b 100644 --- a/common/cross_os_debug/xdbg.cpp +++ b/common/cross_os_debug/xdbg.cpp @@ -488,7 +488,7 @@ bool check_stopped(const ThreadID& tid, SignalInfo* out) { break; default: out->kind = SignalInfo::EXCEPTION; - out->msg = fmt::format("{} [0x{:X}]", exc, win32_exception_code_to_charp(exc)); + out->msg = fmt::format("{} [0x{:X}]", win32_exception_code_to_charp(exc), exc); break; } } diff --git a/common/util/FileUtil.cpp b/common/util/FileUtil.cpp index 9a4e52e189..7a9398e928 100644 --- a/common/util/FileUtil.cpp +++ b/common/util/FileUtil.cpp @@ -92,6 +92,7 @@ void write_binary_file(const std::string& name, const void* data, size_t size) { } if (fwrite(data, size, 1, fp) != 1) { + fclose(fp); throw std::runtime_error("couldn't write file " + name); } @@ -147,6 +148,7 @@ std::vector read_binary_file(const std::string& filename) { data.resize(len); if (fread(data.data(), len, 1, fp) != 1) { + fclose(fp); throw std::runtime_error("File " + filename + " cannot be read"); } fclose(fp); diff --git a/docs/markdown/debugging.md b/docs/markdown/debugging.md index 5ee086bd7b..afe88e0429 100644 --- a/docs/markdown/debugging.md +++ b/docs/markdown/debugging.md @@ -212,23 +212,22 @@ Disassembly instructions in memory Example (after doing a `(lt)`, `(blg)`, `(dbg)`): ```nasm -gc> (:disasm (sym-val basic-type?) 80) -[0x2000162ae4] mov eax, [r15+rdi*1-0x04] -[0x2000162ae9] mov ecx, [r15+r14*1+0x38] -[0x2000162af1] mov rdx, rax -[0x2000162af4] cmp rdx, rsi -[0x2000162af7] jnz 0x0000002000162B0F -[0x2000162afd] mov eax, [r15+r14*1+0x08] -[0x2000162b05] jmp 0x0000002000162B32 -[0x2000162b0a] jmp 0x0000002000162B19 -[0x2000162b0f] mov rax, r14 -[0x2000162b12] add rax, 0x00 -[0x2000162b19] mov eax, [r15+rdx*1+0x04] -[0x2000162b1e] mov rdx, rax -[0x2000162b21] cmp rax, rcx -[0x2000162b24] jnz 0x0000002000162AF4 -[0x2000162b2a] mov eax, [r15+r14*1] -[0x2000162b32] ret +gs> (:disasm (sym-val basic-type?) 59) +Object: gcommon + +[0x28eb4c631c4] mov r9d, [r15+rdi*1-0x04] +[0x28eb4c631c9] mov r8d, [object] +[0x28eb4c631d1] cmp r9, rsi +[0x28eb4c631d4] jnz 0x0000028EB4C631E9 +[0x28eb4c631da] lea rax, '#t +[0x28eb4c631df] jmp 0x0000028EB4C631FD +[0x28eb4c631e4] jmp 0x0000028EB4C631EC +[0x28eb4c631e9] mov rcx, '#f +[0x28eb4c631ec] mov r9d, [r15+r9*1+0x04] +[0x28eb4c631f1] cmp r9, r8 +[0x28eb4c631f4] jnz 0x0000028EB4C631D1 +[0x28eb4c631fa] mov rax, '#f +[0x28eb4c631fd] ret ``` For now, the disassembly is pretty basic, but it should eventually support GOAL symbols. @@ -259,7 +258,7 @@ symbol name for symbol 30h is type gs> (:sym-name #x80) symbol name for symbol 80h is int64 gs> (:sym-name #x800) -symbol name for symbol 800h is +symbol 800h is not loaded or is invalid ``` Keep in mind `-#xa8` is not valid syntax for a negative number in hexadecimal. diff --git a/game/kernel/kprint.cpp b/game/kernel/kprint.cpp index 4681a380dd..354970835e 100644 --- a/game/kernel/kprint.cpp +++ b/game/kernel/kprint.cpp @@ -119,13 +119,19 @@ void clear_print() { */ void reset_output() { if (MasterDebug) { - // original GOAL: - // sprintf(OutputBufArea.cast().c() + sizeof(ListenerMessageHeader), "reset #x%x\n", - // s7.offset); +// original GOAL: +// sprintf(OutputBufArea.cast().c() + sizeof(ListenerMessageHeader), "reset #x%x\n", +// s7.offset); - // modified for OpenGOAL: +// modified for OpenGOAL: +#ifdef _WIN32 + sprintf(OutputBufArea.cast().c() + sizeof(ListenerMessageHeader), + "reset #x%x #x%llx %s\n", s7.offset, (unsigned long long)g_ee_main_mem, // grr + xdbg::get_current_thread_id().to_string().c_str()); +#else sprintf(OutputBufArea.cast().c() + sizeof(ListenerMessageHeader), "reset #x%x #x%lx %s\n", s7.offset, (uintptr_t)g_ee_main_mem, xdbg::get_current_thread_id().to_string().c_str()); +#endif OutputPending = OutputBufArea + sizeof(ListenerMessageHeader); } } diff --git a/goalc/compiler/compilation/Debug.cpp b/goalc/compiler/compilation/Debug.cpp index e2d654da90..b5225ca6dc 100644 --- a/goalc/compiler/compilation/Debug.cpp +++ b/goalc/compiler/compilation/Debug.cpp @@ -338,9 +338,6 @@ Val* Compiler::compile_disasm(const goos::Object& form, const goos::Object& rest ":disasm used on over 1 MB of memory, this probably isn't what you meant to do.")); } - std::vector mem; - mem.resize(size); - if (addr < EE_MAIN_MEM_LOW_PROTECT || (addr + size) > EE_MAIN_MEM_SIZE) { throw_compiler_error(form, ":disasm memory out of range. Wanted to print 0x{:x} to 0x{:x}, but valid " @@ -348,11 +345,8 @@ Val* Compiler::compile_disasm(const goos::Object& form, const goos::Object& rest addr, addr + size, EE_MAIN_MEM_LOW_PROTECT, EE_MAIN_MEM_SIZE); } - m_debugger.read_memory(mem.data(), size, addr); - fmt::print("{}\n", m_debugger.get_info_about_addr(addr)); - fmt::print("{}\n", - disassemble_x86(mem.data(), mem.size(), m_debugger.get_x86_base_addr() + addr)); + fmt::print("{}\n", m_debugger.disassemble_x86_with_symbols(size, addr)); return get_none(); } @@ -403,8 +397,12 @@ Val* Compiler::compile_d_sym_name(const goos::Object& form, const goos::Object& "the target must be halted."); } - fmt::print("symbol name for symbol {:X}h is {}\n", ofs, - m_debugger.get_symbol_name_from_offset(ofs)); + auto sym_name = m_debugger.get_symbol_name_from_offset(ofs); + if (sym_name) { + fmt::print("symbol name for symbol {:X}h is {}\n", ofs, sym_name); + } else { + fmt::print("symbol {:X}h is not loaded or is invalid\n", ofs); + } return get_none(); } diff --git a/goalc/debugger/Debugger.cpp b/goalc/debugger/Debugger.cpp index 81ad3df7fb..b3ae4d34a3 100644 --- a/goalc/debugger/Debugger.cpp +++ b/goalc/debugger/Debugger.cpp @@ -215,7 +215,8 @@ std::vector Debugger::get_backtrace(u64 rip, u64 rsp) { } while (true) { - fmt::print(" rsp: 0x{:x} rip: 0x{:x}\n", rsp, rip); + fmt::print(" rsp: 0x{:x} (#x{:x}) rip: 0x{:x} (#x{:x})\n", rsp, rsp - m_debug_context.base, + rip, rip - m_debug_context.base); BacktraceFrame frame; frame.rip_info = get_rip_info(rip); frame.rsp_at_rip = rsp; @@ -239,16 +240,25 @@ std::vector Debugger::get_backtrace(u64 rip, u64 rsp) { } else { if (!frame.rip_info.knows_function) { - fmt::print("Unknown Function at 0x{:x}\n", rip); - break; - } - if (!frame.rip_info.func_debug) { + fmt::print("Unknown Function at 0x{:x} (#x{:x})\n", rip, rip - m_debug_context.base); + + // attempt to backtrace anyway! if this fails then rip + u64 next_rip = 0; + if (!read_memory_if_safe(&next_rip, rsp - m_debug_context.base)) { + fmt::print("Invalid return address encountered!\n"); + break; + } + + rip = next_rip; + rsp = rsp + 8; // 8 for the call itself. + // break; + } else if (!frame.rip_info.func_debug) { fmt::print("Function {} has no debug info.\n", frame.rip_info.function_name); break; } else { fmt::print("Function {} with no stack frame data.\n", frame.rip_info.function_name); + break; } - break; } bt.push_back(frame); @@ -562,15 +572,15 @@ bool Debugger::get_symbol_value(const std::string& sym_name, u32* output) { } /*! - * Get the value of a symbol by name. Returns if the symbol exists and populates output if it does. + * Get the value of a symbol by name. Returns NULL if symbol does not exist. */ -const char* Debugger::get_symbol_name_from_offset(s32 ofs) { +const char* Debugger::get_symbol_name_from_offset(s32 ofs) const { assert(is_valid()); auto kv = m_symbol_offset_to_name_map.find(ofs); if (kv != m_symbol_offset_to_name_map.end()) { return kv->second.c_str(); } - return ""; + return NULL; } /*! @@ -846,3 +856,72 @@ DebugInfo& Debugger::get_debug_info_for_object(const std::string& object_name) { bool Debugger::knows_object(const std::string& object_name) const { return m_debug_info.find(object_name) != m_debug_info.end(); } + +/*! + * Do x86 disassembly at the specified address and then do some basic string replacement for + * symbols. It will attempt to detect symbol dereferences (e.g. *active-pool*), symbol references + * (e.g. 'dead), and a special case to detect #f (outputted as '#f for correctness). + */ +std::string Debugger::disassemble_x86_with_symbols(int len, u64 base_addr) const { + std::vector mem; + mem.resize(len); + + read_memory(mem.data(), len, base_addr); + + auto result = disassemble_x86(mem.data(), mem.size(), get_x86_base_addr() + base_addr); + + // find symbol values! + const std::string sym_val_string("[r15+r14*1"); + size_t pos = 0; + while ((pos = result.find(sym_val_string, pos)) != std::string::npos) { + size_t read; + auto sym_addr = std::stol(result.substr(pos + sym_val_string.length(), 7), &read, + 16); // -0x1234 is 7 characters + + auto sym_name = get_symbol_name_from_offset((s32)sym_addr); + if (sym_name) { + std::string sym_str(sym_name); + result.replace(pos + 1, read + sym_val_string.length() - 1, + sym_str); // the [ is ignored (result is something like: [identity]) + pos += sym_str.length() + 1; + assert(result.at(pos) == ']'); // maybe? + } else { + // symbol not found for whatever reason, just use regular disassembly and skip over + pos += 1; + } + } + + // find symbol references! + const std::string sym_addr_string("[r14"); + pos = 0; + while ((pos = result.find(sym_addr_string, pos)) != std::string::npos) { + size_t read; + auto sym_addr = std::stol(result.substr(pos + sym_addr_string.length(), 7), &read, + 16); // -0x1234 is 7 characters + + auto sym_name = get_symbol_name_from_offset((s32)sym_addr); + if (sym_name) { + std::string sym_str(sym_name); + result.replace(pos, read + sym_addr_string.length() + 1, fmt::format("'{}", sym_str)); + pos += sym_str.length(); + } else { + // symbol not found for whatever reason, just use regular disassembly and skip over + pos += 1; + } + } + + // find #f references! + const std::string op_mov_string("] mov "); + const std::string sym_false_string(", r14"); + pos = 0; + while ((pos = result.find(op_mov_string, pos)) != std::string::npos) { + pos += op_mov_string.length(); + auto r14_pos = result.find(sym_false_string, pos); + if (r14_pos < result.find(op_mov_string, pos)) { + result.replace(r14_pos, sym_false_string.length(), + fmt::format(", '{}", get_symbol_name_from_offset(0))); + } + } + + return result; +} diff --git a/goalc/debugger/Debugger.h b/goalc/debugger/Debugger.h index 7ac2100161..506103e910 100644 --- a/goalc/debugger/Debugger.h +++ b/goalc/debugger/Debugger.h @@ -87,7 +87,7 @@ class Debugger { void read_symbol_table(); u32 get_symbol_address(const std::string& sym_name); bool get_symbol_value(const std::string& sym_name, u32* output); - const char* get_symbol_name_from_offset(s32 ofs); + const char* get_symbol_name_from_offset(s32 ofs) const; void add_addr_breakpoint(u32 addr); void remove_addr_breakpoint(u32 addr); void update_break_info(); @@ -101,6 +101,8 @@ class Debugger { std::vector get_backtrace(u64 rip, u64 rsp); + std::string disassemble_x86_with_symbols(int len, u64 base_addr) const; + /*! * Get the x86 address of GOAL memory */