[debugger] Improve symbol substitution

Author: drhelius

platforms/shared/desktop/gui_debug_disassembler.cpp

@@ -37,6 +37,7 @@ struct DisassemblerLine
     bool is_breakpoint;
     GG_Disassembler_Record* record;
     char name_enhanced[64];
+    char tooltip[128];
     int name_real_length;
     DebugSymbol* symbol;
     bool is_auto_symbol;
@@ -93,7 +94,7 @@ static void add_auto_symbol(GG_Disassembler_Record* record, u16 address);
 static void add_breakpoint(int type);
 static void request_goto_address(u16 addr);
 static bool is_return_instruction(u8 opcode);
-static void replace_symbols(DisassemblerLine* line, const char* color, const char* auto_color);
+static void replace_symbols(DisassemblerLine* line, const char* jump_color, const char* operand_color, const char* auto_color, const char* original_color);
 static void replace_labels(DisassemblerLine* line, const char* color, const char* original_color);
 static void draw_instruction_name(DisassemblerLine* line, bool is_pc);
 static void disassembler_menu(void);
@@ -636,6 +637,7 @@ static void prepare_drawable_lines(void)
             line.is_breakpoint = false;
             line.record = record;
             snprintf(line.name_enhanced, 64, "%s", line.record->name);
+            line.tooltip[0] = 0;
 
             std::vector<HuC6280::GG_Breakpoint>* breakpoints = emu_get_core()->GetHuC6280()->GetBreakpoints();
 
@@ -805,6 +807,13 @@ static void draw_disassembly(void)
                 ImGui::SameLine();
                 draw_instruction_name(&line, line.address == pc);
 
+                if (line.tooltip[0] != 0 && ImGui::IsItemHovered())
+                {
+                    ImGui::BeginTooltip();
+                    TextColoredEx("%s", line.tooltip);
+                    ImGui::EndTooltip();
+                }
+
                 if (config_debug.dis_show_mem)
                 {
                     int len = line.name_real_length;
@@ -971,6 +980,20 @@ static void add_symbol(const char* line)
                 snprintf(s.text, 64, "%s", symbol.c_str());
 
                 // Store the symbol
+                DebugSymbol* existing = fixed_symbols[s.bank][s.address];
+                if (IsValidPointer(existing))
+                {
+                    for (size_t i = 0; i < fixed_symbol_list.size(); i++)
+                    {
+                        if (fixed_symbol_list[i].symbol == existing)
+                        {
+                            fixed_symbol_list.erase(fixed_symbol_list.begin() + i);
+                            break;
+                        }
+                    }
+                    SafeDelete(fixed_symbols[s.bank][s.address]);
+                }
+
                 DebugSymbol* new_symbol = new DebugSymbol;
                 new_symbol->address = s.address;
                 new_symbol->bank = s.bank;
@@ -1088,81 +1111,169 @@ static bool is_return_instruction(u8 opcode)
     }
 }
 
-static void replace_symbols(DisassemblerLine* line, const char* color, const char* auto_color)
+static bool replace_address_in_string(std::string& instr, u16 address, bool is_zp, const char* replacement_text)
 {
-    bool symbol_found = false;
+    const char* format = is_zp ? "$%02X" : "$%04X";
+    const char* indirect_format = is_zp ? "$(%02X" : "$(%04X";
+    int replace_len = is_zp ? 3 : 5;
+    int indirect_replace_len = is_zp ? 4 : 6;
+
+    char address_str[8];
+    snprintf(address_str, 8, format, address);
+    size_t pos = instr.find(address_str);
+    if (pos != std::string::npos)
+    {
+        instr.replace(pos, replace_len, replacement_text);
+        return true;
+    }
 
-    DebugSymbol* fixed_symbol = fixed_symbols[line->record->jump_bank][line->record->jump_address];
+    snprintf(address_str, 8, indirect_format, address);
+    pos = instr.find(address_str);
+    if (pos != std::string::npos)
+    {
+        std::string indirect_replacement = std::string("(") + replacement_text;
+        instr.replace(pos, indirect_replace_len, indirect_replacement);
+        return true;
+    }
+
+    return false;
+}
 
-    if (IsValidPointer(fixed_symbol))
+static bool get_record_operand(GG_Disassembler_Record* record, u16* out_address, bool* out_is_zp)
+{
+    if (record->jump)
     {
-        std::string instr = line->record->name;
-        std::string symbol = fixed_symbol->text;
-        char jump_address[6];
-        snprintf(jump_address, 6, "$%04X", line->record->jump_address);
-        size_t pos = instr.find(jump_address);
-        if (pos != std::string::npos)
+        *out_address = record->jump_address;
+        *out_is_zp = false;
+        return true;
+    }
+    else if (record->has_operand_address)
+    {
+        *out_address = record->operand_address;
+        *out_is_zp = record->operand_is_zp;
+        return true;
+    }
+    return false;
+}
+
+bool gui_debug_resolve_symbol(GG_Disassembler_Record* record, std::string& instr, const char* color, const char* original_color, const char** out_name, u16* out_address)
+{
+    u16 lookup_address = 0;
+    bool is_zp = false;
+
+    if (!get_record_operand(record, &lookup_address, &is_zp))
+        return false;
+
+    u16 bank_address = is_zp ? (0x2000 | lookup_address) : lookup_address;
+    u8 bank = record->jump ? record->jump_bank : emu_get_core()->GetMemory()->GetBank(bank_address);
+    DebugSymbol* symbol = fixed_symbols[bank][bank_address];
+    if (IsValidPointer(symbol))
+    {
+        std::string replacement = std::string(color) + symbol->text + original_color;
+        if (replace_address_in_string(instr, lookup_address, is_zp, replacement.c_str()))
         {
-            instr.replace(pos, 5, color + symbol);
-            snprintf(line->name_enhanced, 64, "%s", instr.c_str());
-            symbol_found = true;
+            if (out_name) *out_name = symbol->text;
+            if (out_address) *out_address = lookup_address;
+            return true;
         }
     }
 
-    if (symbol_found)
-        return;
-
-    if (!config_debug.dis_show_auto_symbols)
-        return;
+    return false;
+}
 
-    DebugSymbol* dynamic_symbol = dynamic_symbols[line->record->jump_bank][line->record->jump_address];
+bool gui_debug_resolve_label(GG_Disassembler_Record* record, std::string& instr, const char* color, const char* original_color, const char** out_name, u16* out_address)
+{
+    u16 lookup_address = 0;
+    bool is_zp = false;
 
-    if (IsValidPointer(dynamic_symbol))
+    if (get_record_operand(record, &lookup_address, &is_zp))
     {
-        std::string instr = line->record->name;
-        std::string symbol = dynamic_symbol->text;
-        char jump_address[6];
-        snprintf(jump_address, 6, "$%04X", line->record->jump_address);
-        size_t pos = instr.find(jump_address);
-        if (pos != std::string::npos)
+        u16 hardware_offset = 0x0000;
+
+        for (int i = 0; i < 8; i++)
         {
-            instr.replace(pos, 5, auto_color + symbol);
-            snprintf(line->name_enhanced, 64, "%s", instr.c_str());
+            if (emu_get_core()->GetMemory()->GetMpr(i) == 0xFF)
+            {
+                hardware_offset = i * 0x2000;
+                break;
+            }
         }
 
+        u16 label_lookup = is_zp ? (0x2000 | lookup_address) : lookup_address;
+
+        for (int i = 0; i < k_debug_label_count; i++)
+        {
+            if (k_debug_labels[i].address + hardware_offset == label_lookup)
+            {
+                char label_address[6];
+                snprintf(label_address, 6, "$%04X", lookup_address);
+                std::string replacement = std::string(color) + k_debug_labels[i].label + label_address + original_color;
+                if (replace_address_in_string(instr, lookup_address, is_zp, replacement.c_str()))
+                {
+                    if (out_name) *out_name = k_debug_labels[i].label;
+                    if (out_address) *out_address = lookup_address;
+                    return true;
+                }
+            }
+        }
     }
+
+    return false;
 }
 
-static void replace_labels(DisassemblerLine* line, const char* color, const char* original_color)
+static void replace_symbols(DisassemblerLine* line, const char* jump_color, const char* operand_color, const char* auto_color, const char* original_color)
 {
-    u16 hardware_offset = 0x0000;
+    std::string instr = line->record->name;
+    const char* color = line->record->jump ? jump_color : operand_color;
+    const char* resolved_name = NULL;
+    u16 resolved_address = 0;
 
-    for (int i = 0; i < 8; i++)
+    if (gui_debug_resolve_symbol(line->record, instr, color, original_color, &resolved_name, &resolved_address))
     {
-        if (emu_get_core()->GetMemory()->GetMpr(i) == 0xFF)
-        {
-            hardware_offset = i * 0x2000;
-            break;
-        }
+        snprintf(line->name_enhanced, 64, "%s", instr.c_str());
+        snprintf(line->tooltip, 128, "%s%s%s = %s$%04X", color, resolved_name, c_white, c_cyan, resolved_address);
+        return;
     }
 
-    for (int i = 0; i < k_debug_label_count; i++)
+    if (!config_debug.dis_show_auto_symbols)
+        return;
+
+    if (!line->record->jump)
+        return;
+
+    u16 lookup_address = 0;
+    bool is_zp = false;
+
+    if (!get_record_operand(line->record, &lookup_address, &is_zp))
+        return;
+
+    DebugSymbol* dynamic_symbol = dynamic_symbols[line->record->jump_bank][lookup_address];
+
+    if (IsValidPointer(dynamic_symbol))
     {
-        std::string instr = line->record->name;
-        std::string label = k_debug_labels[i].label;
-        char label_address[7];
-        snprintf(label_address, 7, "$%04X", k_debug_labels[i].address + hardware_offset);
-        size_t pos = instr.find(label_address);
-        if (pos != std::string::npos)
+        std::string replacement = std::string(auto_color) + dynamic_symbol->text + original_color;
+        if (replace_address_in_string(instr, lookup_address, is_zp, replacement.c_str()))
         {
-            if (pos > 0 && instr[pos - 1] == '#')
-                continue;
-            instr.replace(pos, 5, color + label + label_address + original_color);
             snprintf(line->name_enhanced, 64, "%s", instr.c_str());
+            snprintf(line->tooltip, 128, "%s%s%s = %s$%04X", auto_color, dynamic_symbol->text, c_white, c_cyan, lookup_address);
         }
     }
 }
 
+static void replace_labels(DisassemblerLine* line, const char* color, const char* original_color)
+{
+    std::string instr = line->record->name;
+    const char* resolved_name = NULL;
+    u16 resolved_address = 0;
+
+    if (gui_debug_resolve_label(line->record, instr, color, original_color, &resolved_name, &resolved_address))
+    {
+        snprintf(line->name_enhanced, 64, "%s", instr.c_str());
+        if (line->tooltip[0] == 0)
+            snprintf(line->tooltip, 128, "%s%s%s = %s$%04X", color, resolved_name, c_white, c_cyan, resolved_address);
+    }
+}
+
 static void draw_instruction_name(DisassemblerLine* line, bool is_pc)
 {
     const char* name_color;
@@ -1196,10 +1307,10 @@ static void draw_instruction_name(DisassemblerLine* line, bool is_pc)
         extra_color = c_blue;
     }
 
-    if (config_debug.dis_replace_symbols && line->record->jump)
+    if (config_debug.dis_replace_symbols)
     {
         const char* auto_symbol_color = config_debug.dis_dim_auto_symbols ? c_dim_green : symbol_color;
-        replace_symbols(line, symbol_color, auto_symbol_color);
+        replace_symbols(line, symbol_color, label_color, auto_symbol_color, operands_color);
     }
 
     if (config_debug.dis_replace_labels)
@@ -1218,7 +1329,9 @@ static void draw_instruction_name(DisassemblerLine* line, bool is_pc)
     if (pos != std::string::npos)
         instr.replace(pos, 3, operands_color);
 
+    ImGui::BeginGroup();
     line->name_real_length = TextColoredEx("%s%s", name_color, instr.c_str());
+    ImGui::EndGroup();
 }
 
 static void disassembler_menu(void)
@@ -1437,13 +1550,13 @@ static void disassembler_menu(void)
         ImGui::MenuItem("Symbols Window", NULL, &config_debug.show_symbols);
 
         ImGui::Separator();
+        ImGui::MenuItem("Hardware Labels", NULL, &config_debug.dis_replace_labels);
 
         ImGui::MenuItem("Automatic Symbols", NULL, &config_debug.dis_show_auto_symbols);
         if (!config_debug.dis_show_auto_symbols) ImGui::BeginDisabled();
         ImGui::MenuItem("Dim Automatic Symbols", NULL, &config_debug.dis_dim_auto_symbols);
         if (!config_debug.dis_show_auto_symbols) ImGui::EndDisabled();
         ImGui::MenuItem("Replace Address With Symbol", NULL, &config_debug.dis_replace_symbols);
-        ImGui::MenuItem("Replace Address With Label", NULL, &config_debug.dis_replace_labels);
 
         ImGui::Separator();
 
@@ -1869,6 +1982,14 @@ void gui_debug_window_symbols(void)
                             emu_get_core()->GetHuC6280()->AddBreakpoint(symbol->address);
                     }
 
+                    if (is_manual)
+                    {
+                        if (ImGui::Selectable("Remove Symbol"))
+                        {
+                            gui_debug_remove_symbol(b, symbol->address);
+                        }
+                    }
+
                     ImGui::EndPopup();
                 }
                 ImGui::PushFont(gui_default_font);
@@ -1906,26 +2027,20 @@ void gui_debug_add_symbol(const char* symbol_str)
 
 void gui_debug_remove_symbol(u8 bank, u16 address)
 {
-    Memory* memory = emu_get_core()->GetMemory();
-    GG_Disassembler_Record* record = memory->GetDisassemblerRecord(address);
-
-    if (IsValidPointer(record) && record->bank == bank)
+    DebugSymbol* symbol = fixed_symbols[bank][address];
+    if (IsValidPointer(symbol))
     {
-        DebugSymbol* symbol = fixed_symbols[bank][address];
-        if (IsValidPointer(symbol))
+        for (size_t i = 0; i < fixed_symbol_list.size(); i++)
         {
-            for (size_t i = 0; i < fixed_symbol_list.size(); i++)
+            if (fixed_symbol_list[i].symbol == symbol)
             {
-                if (fixed_symbol_list[i].symbol == symbol)
-                {
-                    fixed_symbol_list.erase(fixed_symbol_list.begin() + i);
-                    break;
-                }
+                fixed_symbol_list.erase(fixed_symbol_list.begin() + i);
+                break;
             }
-            delete symbol;
-            fixed_symbols[bank][address] = NULL;
-            symbols_dirty = true;
         }
+        delete symbol;
+        fixed_symbols[bank][address] = NULL;
+        symbols_dirty = true;
     }
 }
 
@@ -2048,9 +2163,9 @@ static void save_current_disassembler(FILE* file)
 
         fprintf(file, " %04X ", line.address);
 
-        if (config_debug.dis_replace_symbols && line.record->jump)
+        if (config_debug.dis_replace_symbols)
         {
-            replace_symbols(&line, "", "");
+            replace_symbols(&line, "", "", "", "");
         }
 
         if (config_debug.dis_replace_labels)

platforms/shared/desktop/gui_debug_disassembler.h

@@ -21,6 +21,7 @@
 #define GUI_DEBUG_DISASSEMBLER_H
 
 #include "geargrafx.h"
+#include <string>
 
 #ifdef GUI_DEBUG_DISASSEMBLER_IMPORT
     #define EXTERN
@@ -51,6 +52,8 @@ EXTERN void gui_debug_remove_disassembler_bookmark(u16 address);
 EXTERN int gui_debug_get_disassembler_bookmarks(void** bookmarks_ptr);
 EXTERN void gui_debug_reset_disassembler_bookmarks(void);
 EXTERN int gui_debug_get_symbols(void** symbols_ptr);
+EXTERN bool gui_debug_resolve_symbol(GG_Disassembler_Record* record, std::string& instr, const char* color, const char* original_color, const char** out_name = NULL, u16* out_address = NULL);
+EXTERN bool gui_debug_resolve_label(GG_Disassembler_Record* record, std::string& instr, const char* color, const char* original_color, const char** out_name = NULL, u16* out_address = NULL);
 EXTERN void gui_debug_runtocursor(void);
 EXTERN void gui_debug_runto_address(u16 address);
 EXTERN void gui_debug_go_back(void);