| Re: [hatari-devel] debugger suggestions |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
Hi,
On 25.4.2022 18.02, J.Young wrote:
I am wondering if the following breakpoint conditions can be added:
1) break on memory address access rw (b, w or l).
This is a common wish.
Nicolas?
(It would need CPU core to update variables that store last read and
write access addresses, and a bit of code on debugger side to to support
setting breakpoints on their values, and to zero their values before
continuing emulation from debugger.)
2) break on *any* loaded symbol not just a specific one.
First of the attached patches does that. Please test.
Also general debugger requests:
1) when disassembling at an address, sometimes I want to peek at the
code above.
could an option like 'd -<number of byte/instructions>' or just 'd -'
for a page above be added to allow moving back a bit in the disassembly?
You can already do that with:
d "pc-<bytes>"
The problem is that disassembly started at random offset can produce
garbage. There's no way to backtrack / sync instruction address either,
as long as one does not know what those instructions are.
If you enable debugger history, you get that information, if those
instructions were executed before debugger was entered.
Second patch adds debugger support for taking default disassembly
address (when re-entering debugger) from execution history when history
is enabled.
Please test and comment on that too.
2) Also, when disassembling consider this code segment:
new_call:
0008DD84 2f0a MOVE.L A2,-(A7) [00000000]
0008DD86 2f0b MOVE.L A3,-(A7) [00000000]
0008DD88 2648 MOVEA.L A0,A3
0008DD8A 2039 0011 6e8a MOVE.L $00116e8a [00000000],D0
0008DD90 670c BEQ.B #$0c == $00000000 (F)
0008DD92 2240 MOVEA.L D0,A1
0008DD94 2451 MOVEA.L (A1) [602e0162],A2
0008DD96 204a MOVEA.L A2,A0
0008DD98 224b MOVEA.L A3,A1
0008DD9A 6100 fe26 BSR.W #$fe26 == $0008dbc2
The last line references $0008dbc2 which actually has a symbol:
open_files:
0008DBC2 48e7 0038 MOVEM.L A2-A4,-(A7)
Request the disassembler to replace the addresses with symbols if
available.
While that would be nice, there are several issues:
* Hatari has currently 2 disassemblers, one provided by the CPU core
itself, and "ext" one
* Nowadays disassembler in (Amiga) WinUAE CPU core is more important (it
has better instruction support), but that is a separate project from Hatari
Toni (maintaining WinUAE) would need to add callback support to WinUAE
disassembler for replacing addresses with corresponding symbol names.
Then after WinUAE disassembler has all the features provided by "ext"
disassembler, we could drop the "ext" one.
- Eero
From 5350318d9cf03c47b58ae4c8229fa625a07628f1 Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Tue, 21 Sep 2021 00:01:54 +0300
Subject: [PATCH 2/2] Use history for determining suitable disassembly default
address
If disassembly starts from program counter (PC), user lacks context on
how the code ended in current position.
Because trying to disassemble instructions from "random" address will
often output incorrect disassembly, this scans execution history for a
valid instruction address that's close enough to PC.
History addresses are valid until overwritten with differently aligned
instructions, which is extremely unlike to happen, so this should be
robust enough to be used always when history is enabled.
---
doc/debugger.html | 11 ++++++--
doc/release-notes.txt | 3 +++
src/debug/debugcpu.c | 3 ++-
src/debug/debugdsp.c | 3 ++-
src/debug/history.c | 59 ++++++++++++++++++++++++++++++++++++++-----
src/debug/history.h | 3 ++-
6 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/doc/debugger.html b/doc/debugger.html
index fb10a82c..e846180c 100644
--- a/doc/debugger.html
+++ b/doc/debugger.html
@@ -330,6 +330,7 @@ Usage: d [start address-[end address]]
</pre>
<pre>
> disasm pc
+(PC)
$00aa6e : 2f08 move.l a0,-(sp)
$00aa70 : 0241 0fff andi.w #$fff,d1
$00aa74 : 207c 00fe 78c0 movea.l #$fe78c0,a0
@@ -341,8 +342,14 @@ $00aa7e : 4ed0 jmp (a0)
Both commands accept in addition to numeric addresses also register
and symbol names, like in above example. If you don't specify an
address, the commands continue showing from an address that comes
-after the previously shown data. "disasm" command default address
-will be reset to PC address every time you re-enter the debugger.
+after the previously shown data.
+</p>
+
+<p>
+"disasm" command default address will be reset to program counter (PC)
+address every time you re-enter the debugger. If history is enabled
+and it includes addresses just before PC, disassembly will instead
+start from such an address to give more context.
</p>
<p>
diff --git a/doc/release-notes.txt b/doc/release-notes.txt
index 559e5480..d36ec902 100644
--- a/doc/release-notes.txt
+++ b/doc/release-notes.txt
@@ -71,6 +71,9 @@ Emulator improvements:
- Debugger:
- Repeat CPU & DSP "step" and "next" commands on Enter
- Output PC location in CPU & DSP disassembly
+ - When history is enabled, disassembly address defaults to an
+ address in history preceeding PC reg value (instead of to PC),
+ when (re-)entering debugger
- More readable "variables" command output
- Add "mmu" and "vme" options for "info" command
- Add "scc" and "vme" (VME/SCU reg access) tracing support
diff --git a/src/debug/debugcpu.c b/src/debug/debugcpu.c
index 37e978d1..6114d0d6 100644
--- a/src/debug/debugcpu.c
+++ b/src/debug/debugcpu.c
@@ -1075,6 +1075,7 @@ int DebugCpu_Init(const dbgcommand_t **table)
*/
void DebugCpu_InitSession(void)
{
- disasm_addr = M68000_GetPC();
+#define MAX_CPU_DISASM_OFFSET 16
+ disasm_addr = History_DisasmAddr(M68000_GetPC(), MAX_CPU_DISASM_OFFSET, false);
Profile_CpuStop();
}
diff --git a/src/debug/debugdsp.c b/src/debug/debugdsp.c
index 140c96e5..e0abcb83 100644
--- a/src/debug/debugdsp.c
+++ b/src/debug/debugdsp.c
@@ -672,6 +672,7 @@ int DebugDsp_Init(const dbgcommand_t **table)
*/
void DebugDsp_InitSession(void)
{
- dsp_disasm_addr = DSP_GetPC();
+#define MAX_DSP_DISASM_OFFSET 8
+ dsp_disasm_addr = (Uint16)History_DisasmAddr(DSP_GetPC(), MAX_DSP_DISASM_OFFSET, true);
Profile_DspStop();
}
diff --git a/src/debug/history.c b/src/debug/history.c
index bc170dd8..82f6a610 100644
--- a/src/debug/history.c
+++ b/src/debug/history.c
@@ -13,6 +13,7 @@ const char History_fileid[] = "Hatari history.c";
#include <assert.h>
#include <errno.h>
#include "main.h"
+#include "configuration.h"
#include "debugui.h"
#include "debug_priv.h"
#include "dsp.h"
@@ -157,6 +158,55 @@ void History_Mark(debug_reason_t reason)
}
}
+/**
+ * Find lowest address in history that is within range:
+ * (pc-offset) - pc
+ * where 'offset' is the history disasm offset limit.
+ *
+ * If history has no such address, return given pc value.
+ */
+Uint32 History_DisasmAddr(Uint32 pc, Uint32 offset, bool for_dsp)
+{
+ unsigned int i, count;
+ Uint32 limit, first;
+ int track;
+
+ if (!offset) {
+ return pc;
+ }
+ track = for_dsp ? HISTORY_TRACK_DSP : HISTORY_TRACK_CPU;
+ if (!(track & HistoryTracking)) {
+ return pc;
+ }
+ count = History.count;
+ if (count > History.limit) {
+ count = History.limit;
+ }
+ if (count <= 0) {
+ return pc;
+ }
+ first = pc;
+ limit = pc - offset;
+ i = History.idx + History.limit - count;
+ while (count-- > 0) {
+ i++;
+ i %= History.limit;
+ assert(History.item[i].valid);
+ if (History.item[i].for_dsp != for_dsp) {
+ continue;
+ }
+ if (for_dsp) {
+ pc = History.item[i].pc.dsp;
+ } else {
+ pc = History.item[i].pc.cpu;
+ }
+ if (pc >= limit && pc < first) {
+ first = pc;
+ }
+ }
+ return first;
+}
+
/**
* Output collected CPU/DSP debugger/breakpoint history
*/
@@ -183,20 +233,17 @@ static Uint32 History_Output(Uint32 count, FILE *fp)
}
retval = count;
- i = History.idx;
show_all = false;
- if (History.item[i].shown) {
+ if (History.item[History.idx].shown) {
/* even last item already shown, show all again */
show_all = true;
}
- i = (i + History.limit - count) % History.limit;
+ i = History.idx + History.limit - count;
while (count-- > 0) {
i++;
i %= History.limit;
- if (!History.item[i].valid) {
- fprintf(fp, "ERROR: invalid history item %d!", count);
- }
+ assert(History.item[i].valid);
if (History.item[i].shown && !show_all) {
continue;
}
diff --git a/src/debug/history.h b/src/debug/history.h
index e3da1477..93602768 100644
--- a/src/debug/history.h
+++ b/src/debug/history.h
@@ -13,7 +13,7 @@ typedef enum {
HISTORY_TRACK_NONE = 0,
HISTORY_TRACK_CPU = 1,
HISTORY_TRACK_DSP = 2,
- HISTORY_TRACK_ALL = 3
+ HISTORY_TRACK_ALL = (HISTORY_TRACK_CPU|HISTORY_TRACK_DSP)
} history_type_t;
extern history_type_t HistoryTracking;
@@ -30,6 +30,7 @@ static inline bool History_TrackDsp(void)
/* for debugcpu/dsp.c */
extern void History_AddCpu(void);
extern void History_AddDsp(void);
+extern Uint32 History_DisasmAddr(Uint32 pc, Uint32 offset, bool for_dsp);
/* for debugInfo.c */
extern void History_Show(FILE *fp, Uint32 count);
--
2.30.2
From 5b15c1d757a468152b9db21d80784e2bd0d411c1 Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Wed, 8 Sep 2021 22:58:12 +0300
Subject: [PATCH 1/2] Add "PConSymbol" debugger variable
To trigger breakpoint on each symbols (ones have been added to
the program for that particular purpose).
---
src/debug/vars.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/debug/vars.c b/src/debug/vars.c
index a62b69a8..faff4dff 100644
--- a/src/debug/vars.c
+++ b/src/debug/vars.c
@@ -139,6 +139,19 @@ Uint32 Vars_GetVdiOpcode(void)
return INVALID_OPCODE;
}
+/** return 1 if PC is on Symbol, 0 otherwise
+ */
+static Uint32 PConSymbol(void)
+{
+ const char *sym;
+ Uint32 pc = M68000_GetPC();
+ sym = Symbols_GetByCpuAddress(pc, SYMTYPE_TEXT);
+ if (sym) {
+ return 1;
+ }
+ return 0;
+}
+
/** return first word in OS call parameters
*/
static Uint32 GetOsCallParam(void)
@@ -176,6 +189,7 @@ static const var_addr_t hatari_vars[] = {
{ "LineFOpcode", (Uint32*)GetLineFOpcode, VALUE_TYPE_FUNCTION32, 16, "$FFFF when not on Line-F opcode" },
{ "NextPC", (Uint32*)GetNextPC, VALUE_TYPE_FUNCTION32, 0, "Next instruction address" },
{ "OsCallParam", (Uint32*)GetOsCallParam, VALUE_TYPE_FUNCTION32, 16, "valid only on OS call opcode breakpoint" },
+ { "PConSymbol", (Uint32*)PConSymbol, VALUE_TYPE_FUNCTION32, 16, "1 if PC on symbol, 0 otherwise" },
{ "TEXT", (Uint32*)DebugInfo_GetTEXT, VALUE_TYPE_FUNCTION32, 0, "invalid before Desktop is up" },
{ "TEXTEnd", (Uint32*)DebugInfo_GetTEXTEnd, VALUE_TYPE_FUNCTION32, 0, "invalid before Desktop is up" },
{ "VBL", (Uint32*)&nVBLs, VALUE_TYPE_VAR32, sizeof(nVBLs)*8, "number of VBL interrupts" },
--
2.30.2