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

@@ -31,10 +31,9 @@ if(UNIX)
         -fdiagnostics-color=always")
 else()
     if(CMAKE_BUILD_TYPE MATCHES "Debug")
-      message("Setting Flags to Enable Edit and Continue")
+      # This actually breaks some standard library things for some reason?
-      set(CMAKE_CXX_FLAGS_DEBUG "/ZI")
+      # message("Setting Flags to Enable Edit and Continue")
-      set(CMAKE_SHARED_LINKER_FLAGS "/SAFESEH:NO")
+      # set(CMAKE_CXX_FLAGS_DEBUG "/ZI")
-      set(CMAKE_EXE_LINKER_FLAGS "/SAFESEH:NO")
     endif()
     set(CMAKE_CXX_FLAGS "/EHsc")
     set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:16000000,16384")

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",

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;
           }
         }

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<uint8_t> 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);

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)
+gs> (:disasm (sym-val basic-type?) 59)
-[0x2000162ae4] mov eax, [r15+rdi*1-0x04]
+Object: gcommon
-[0x2000162ae9] mov ecx, [r15+r14*1+0x38]
+
-[0x2000162af1] mov rdx, rax
+[0x28eb4c631c4] mov r9d, [r15+rdi*1-0x04]
-[0x2000162af4] cmp rdx, rsi
+[0x28eb4c631c9] mov r8d, [object]
-[0x2000162af7] jnz 0x0000002000162B0F
+[0x28eb4c631d1] cmp r9, rsi
-[0x2000162afd] mov eax, [r15+r14*1+0x08]
+[0x28eb4c631d4] jnz 0x0000028EB4C631E9
-[0x2000162b05] jmp 0x0000002000162B32
+[0x28eb4c631da] lea rax, '#t
-[0x2000162b0a] jmp 0x0000002000162B19
+[0x28eb4c631df] jmp 0x0000028EB4C631FD
-[0x2000162b0f] mov rax, r14
+[0x28eb4c631e4] jmp 0x0000028EB4C631EC
-[0x2000162b12] add rax, 0x00
+[0x28eb4c631e9] mov rcx, '#f
-[0x2000162b19] mov eax, [r15+rdx*1+0x04]
+[0x28eb4c631ec] mov r9d, [r15+r9*1+0x04]
-[0x2000162b1e] mov rdx, rax
+[0x28eb4c631f1] cmp r9, r8
-[0x2000162b21] cmp rax, rcx
+[0x28eb4c631f4] jnz 0x0000028EB4C631D1
-[0x2000162b24] jnz 0x0000002000162AF4
+[0x28eb4c631fa] mov rax, '#f
-[0x2000162b2a] mov eax, [r15+r14*1]
+[0x28eb4c631fd] ret
-[0x2000162b32] 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 <invalid symbol offset>
+symbol 800h is not loaded or is invalid
 ```
 
 Keep in mind `-#xa8` is not valid syntax for a negative number in hexadecimal.

game/kernel/kprint.cpp

@@ -124,8 +124,14 @@ void reset_output() {
 // s7.offset);
 
 // modified for OpenGOAL:
+#ifdef _WIN32
+    sprintf(OutputBufArea.cast<char>().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<char>().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);
   }
 }

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<u8> 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",
+  fmt::print("{}\n", m_debugger.disassemble_x86_with_symbols(size, addr));
-             disassemble_x86(mem.data(), mem.size(), m_debugger.get_x86_base_addr() + 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,
+  auto sym_name = m_debugger.get_symbol_name_from_offset(ofs);
-             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();
 }

goalc/debugger/Debugger.cpp

@@ -215,7 +215,8 @@ std::vector<BacktraceFrame> 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,17 +240,26 @@ std::vector<BacktraceFrame> Debugger::get_backtrace(u64 rip, u64 rsp) {
 
     } else {
       if (!frame.rip_info.knows_function) {
-        fmt::print("Unknown Function at 0x{:x}\n", rip);
+        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<u64>(&next_rip, rsp - m_debug_context.base)) {
+          fmt::print("Invalid return address encountered!\n");
           break;
         }
-      if (!frame.rip_info.func_debug) {
+
+        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;
       }
+    }
 
     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 "<invalid symbol offset>";
+  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<u8> 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;
+}

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<BacktraceFrame> 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
    */