[hatari-devel] WinUAE CPU core disassembler output options

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi,

Attached patch adds preliminary support for controlling UAE disassembly output.

It allows configuring all CPU core disassembly flags except the DISASM_FLAG_ABSSHORTLONG one for address length of shorts (8 vs 4 hex chars).

This was inspired by Christian's complaint about its too cluttered output at atari-forum, and remembering that Toni added already some output options to CPU core disassembler.

Comments?


The main thing that still makes CPU core disassembly output too cluttered, is opcode mnemonic and its operands being together, whereas external disassembler shows them in separate columns.

Toni, could there be support for separating those?


	- Eero
From 128d44d3eece10d3f9159ff1094242a55feaa7fd Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Fri, 7 Oct 2022 13:26:18 +0300
Subject: [PATCH] Improve init and output options for disassembly

* Support lower case options also with CPU core disassembly
* Add UAE disassembly option for showing EA/CC info
* Add UAE disassembly option for hex address case
* Add option for skipping showing of 16-bit words at address
  - To manage that better, offsets come now from positions array
  - Add const correctness to column removal function
  - Remove " :" after ext assembler address, to make output
    more consistent between dissassembler and because it
    looked odd whent 16-bit words were missing
* Convert Disasm_SetCPUType() to Disasm_Init() function
  that handles intialization for both disassemblers
* Show flag values as hex in help, now that there are more of them
---
 src/configuration.c           |   6 +-
 src/debug/68kDisass.c         | 193 +++++++++++++++++++++++-----------
 src/debug/68kDisass.h         |   4 +-
 tests/debugger/test-dummies.c |   2 +-
 4 files changed, 138 insertions(+), 67 deletions(-)

diff --git a/src/configuration.c b/src/configuration.c
index 4034c765..40987a94 100644
--- a/src/configuration.c
+++ b/src/configuration.c
@@ -547,8 +547,7 @@ void Configuration_SetDefault(void)
 	ConfigureParams.Debugger.bSymbolsAutoLoad = true;
 	ConfigureParams.Debugger.bMatchAllSymbols = false;
 	ConfigureParams.Debugger.nDisasmOptions = Disasm_GetOptions();
-	if ( ConfigureParams.Debugger.bDisasmUAE )
-		disasm_init();
+	Disasm_Init();
 
 	/* Set defaults for floppy disk images */
 	ConfigureParams.DiskImage.bAutoInsertDiskB = true;
@@ -944,8 +943,7 @@ void Configuration_Apply(bool bReset)
 	FDC_Drive_Set_NumberOfHeads ( 1 , ConfigureParams.DiskImage.DriveB_NumberOfHeads );
 
 	/* Update disassembler */
-	Disasm_SetCPUType(ConfigureParams.System.nCpuLevel, ConfigureParams.System.n_FPUType,
-	                  ConfigureParams.System.bMMU);
+	Disasm_Init();
 
 #if ENABLE_DSP_EMU
 	/* Enable DSP ? */
diff --git a/src/debug/68kDisass.c b/src/debug/68kDisass.c
index 9099cdfa..797cce7a 100644
--- a/src/debug/68kDisass.c
+++ b/src/debug/68kDisass.c
@@ -26,25 +26,34 @@
 #include "disasm.h"
 
 typedef enum {
-	doptNoSpace = 1,	// no space after a comma in the operands list
-	doptOpcodesSmall = 2,	// opcodes are small letters
-	doptRegisterSmall = 4,	// register names are small letters
-	doptStackSP = 8		// stack pointer is named "SP" instead of "A7" (except for MOVEM)
+	doptNoSpace       = (1 << 0),	// ext: no space after a comma in the operands list
+	doptOpcodesSmall  = (1 << 1),	// opcodes in lower case
+	doptRegisterSmall = (1 << 2),	// register names in lower case
+	doptStackSP       = (1 << 3),	// ext: stack pointer is named "SP" instead of "A7" (except for MOVEM)
+	doptNoWords       = (1 << 4),	// do no show 16-bit words at this address
+	doptShowValues    = (1 << 5),	// uae: show EA & CC value in disassembly
+	doptHexSmall      = (1 << 6),	// uae: hex addresses in lower case
 } Diss68kOptions;
 
-// Note: doptNoBrackets and doptStackSP are not implemented anymore
+// Note: doptNoBrackets is not implemented anymore
 static Diss68kOptions	options = doptOpcodesSmall | doptRegisterSmall | doptNoSpace;
 
-/* all options */
-static const Diss68kOptions optionsMask = doptOpcodesSmall | doptRegisterSmall | doptStackSP | doptNoSpace;
+/* all options for 'ext' and 'uae' disassemblers */
+#define COMMON_OPTS (doptOpcodesSmall | doptRegisterSmall | doptNoWords)
+static const Diss68kOptions extOptMask = COMMON_OPTS | doptStackSP | doptNoSpace;
+static const Diss68kOptions uaeOptMask = COMMON_OPTS | doptShowValues | doptHexSmall;
 
-// values <0 will hide the group
-static int optionPosAddress = 0;	// current address
-static int optionPosHexdump = 12;	// 16-bit words at this address
-static int optionPosLabel   = 35;	// label, if defined
-static int optionPosOpcode  = 47;	// opcode
-static int optionPosOperand = 57;	// operands for the opcode
-static int optionPosComment = 82;	// comment, if defined
+static const int defaultPositions[DISASM_COLUMNS] = {
+	 0, // address: current address
+	10, // hexdump: 16-bit words at this address
+	33, // label: if defined
+	45, // opcode
+	55, // operands: for the opcode
+	80  // comment: if defined
+};
+
+// DISASM_COLUMN_DISABLE value will skip given column data
+static int positions[DISASM_COLUMNS];
 
 static int optionCPUTypeMask;
 
@@ -212,12 +221,12 @@ static void Disass68k_loop (FILE *f, uaecptr addr, uaecptr *nextpc, int cnt)
 		if (!len)
 			break;
 
-		sprintf(addressBuffer, "$%*.*x :", addrWidth,addrWidth, addr);
+		sprintf(addressBuffer, "$%*.*x", addrWidth,addrWidth, addr);
 
 		hexdumpBuffer[0] = 0;
 		plen = len;
 		if(plen > 80 && (!strncmp(opcodeBuffer, "DC.", 3) || !strncmp(opcodeBuffer, "dc.", 3)))
-			plen = ((optionPosLabel - optionPosHexdump) / 5) * 2;
+			plen = ((positions[DISASM_COLUMN_LABEL] - positions[DISASM_COLUMN_HEXDUMP]) / 5) * 2;
 
 		for(j=0; j<plen; j += 2)
 		{
@@ -234,15 +243,15 @@ static void Disass68k_loop (FILE *f, uaecptr addr, uaecptr *nextpc, int cnt)
 		}
 
 		lineBuffer[0] = 0;
-		if(optionPosAddress >= 0)
-			Disass68kComposeStr(lineBuffer, addressBuffer, optionPosAddress, 0);
-		if(optionPosHexdump >= 0)
-			Disass68kComposeStr(lineBuffer, hexdumpBuffer, optionPosHexdump, optionPosLabel);
-		if(optionPosLabel >= 0)
-			Disass68kComposeStr(lineBuffer, labelBuffer, optionPosLabel, 0);
-		if(optionPosOpcode >= 0)
-			Disass68kComposeStr(lineBuffer, opcodeBuffer, optionPosOpcode, 0);
-		if(optionPosOperand >= 0)
+		if(positions[DISASM_COLUMN_ADDRESS] >= 0)
+			Disass68kComposeStr(lineBuffer, addressBuffer, positions[DISASM_COLUMN_ADDRESS], 0);
+		if(positions[DISASM_COLUMN_HEXDUMP] >= 0)
+			Disass68kComposeStr(lineBuffer, hexdumpBuffer, positions[DISASM_COLUMN_HEXDUMP], positions[DISASM_COLUMN_LABEL]);
+		if(positions[DISASM_COLUMN_LABEL] >= 0)
+			Disass68kComposeStr(lineBuffer, labelBuffer, positions[DISASM_COLUMN_LABEL], 0);
+		if(positions[DISASM_COLUMN_OPCODE] >= 0)
+			Disass68kComposeStr(lineBuffer, opcodeBuffer, positions[DISASM_COLUMN_OPCODE], 0);
+		if(positions[DISASM_COLUMN_OPERAND] >= 0)
 		{
 			size_t	l = strlen(lineBuffer);
 			if(lineBuffer[l-1] != ' ')		// force at least one space between opcode and operand
@@ -250,19 +259,19 @@ static void Disass68k_loop (FILE *f, uaecptr addr, uaecptr *nextpc, int cnt)
 				lineBuffer[l++] = ' ';
 				lineBuffer[l] = 0;
 			}
-			Disass68kComposeStr(lineBuffer, operandBuffer, optionPosOperand, 0);
+			Disass68kComposeStr(lineBuffer, operandBuffer, positions[DISASM_COLUMN_OPERAND], 0);
 		}
-		if (optionPosComment >= 0)
+		if (positions[DISASM_COLUMN_COMMENT] >= 0)
 		{
 			if (Profile_CpuAddr_DataStr(commentBuffer, sizeof(commentBuffer), addr))
 			{
-				Disass68kComposeStr(lineBuffer, commentBuffer, optionPosComment+1, 0);
+				Disass68kComposeStr(lineBuffer, commentBuffer, positions[DISASM_COLUMN_COMMENT]+1, 0);
 			}
 			/* show comments only if profile data is missing */
 			else if (commentBuffer[0])
 			{
-				Disass68kComposeStr(lineBuffer, " ;", optionPosComment, 0);
-				Disass68kComposeStr(lineBuffer, commentBuffer, optionPosComment+3, 0);
+				Disass68kComposeStr(lineBuffer, " ;", positions[DISASM_COLUMN_COMMENT], 0);
+				Disass68kComposeStr(lineBuffer, commentBuffer, positions[DISASM_COLUMN_COMMENT]+3, 0);
 			}
 		}
 		addr += len;
@@ -309,10 +318,26 @@ void Disasm (FILE *f, uaecptr addr, uaecptr *nextpc, int cnt)
 	m68k_disasm_file (f, addr, nextpc, addr, cnt);
 }
 
-static void Disasm_CheckOptionEngine(void)
+/**
+ * warn if flags for the other engine have been specified
+ */
+static void Disasm_CheckOptionEngine(Diss68kOptions opts)
 {
+	const char *name;
+	Diss68kOptions mask;
 	if (ConfigureParams.Debugger.bDisasmUAE)
-		fputs("WARNING: disassembly options are supported only for '--disasm ext'!\n", stderr);
+	{
+		mask = uaeOptMask;
+		name = "uae";
+	}
+	else
+	{
+		mask = extOptMask;
+		name = "ext";
+	}
+	if (opts & ~mask)
+		fprintf(stderr, "WARNING: '--disasm %s' does not support disassembly option(s) 0x%x!\n",
+		       name, opts & ~mask);
 }
 
 /**
@@ -320,12 +345,12 @@ static void Disasm_CheckOptionEngine(void)
  */
 void Disasm_GetColumns(int *pos)
 {
-	pos[DISASM_COLUMN_ADDRESS] = optionPosAddress;
-	pos[DISASM_COLUMN_HEXDUMP] = optionPosHexdump;
-	pos[DISASM_COLUMN_LABEL]   = optionPosLabel;
-	pos[DISASM_COLUMN_OPCODE]  = optionPosOpcode;
-	pos[DISASM_COLUMN_OPERAND] = optionPosOperand;
-	pos[DISASM_COLUMN_COMMENT] = optionPosComment;
+	pos[DISASM_COLUMN_ADDRESS] = positions[DISASM_COLUMN_ADDRESS];
+	pos[DISASM_COLUMN_HEXDUMP] = positions[DISASM_COLUMN_HEXDUMP];
+	pos[DISASM_COLUMN_LABEL]   = positions[DISASM_COLUMN_LABEL];
+	pos[DISASM_COLUMN_OPCODE]  = positions[DISASM_COLUMN_OPCODE];
+	pos[DISASM_COLUMN_OPERAND] = positions[DISASM_COLUMN_OPERAND];
+	pos[DISASM_COLUMN_COMMENT] = positions[DISASM_COLUMN_COMMENT];
 }
 
 /**
@@ -333,12 +358,12 @@ void Disasm_GetColumns(int *pos)
  */
 void Disasm_SetColumns(int *pos)
 {
-	optionPosAddress = pos[DISASM_COLUMN_ADDRESS];
-	optionPosHexdump = pos[DISASM_COLUMN_HEXDUMP];
-	optionPosLabel   = pos[DISASM_COLUMN_LABEL];
-	optionPosOpcode  = pos[DISASM_COLUMN_OPCODE];
-	optionPosOperand = pos[DISASM_COLUMN_OPERAND];
-	optionPosComment = pos[DISASM_COLUMN_COMMENT];
+	positions[DISASM_COLUMN_ADDRESS] = pos[DISASM_COLUMN_ADDRESS];
+	positions[DISASM_COLUMN_HEXDUMP] = pos[DISASM_COLUMN_HEXDUMP];
+	positions[DISASM_COLUMN_LABEL]   = pos[DISASM_COLUMN_LABEL];
+	positions[DISASM_COLUMN_OPCODE]  = pos[DISASM_COLUMN_OPCODE];
+	positions[DISASM_COLUMN_OPERAND] = pos[DISASM_COLUMN_OPERAND];
+	positions[DISASM_COLUMN_COMMENT] = pos[DISASM_COLUMN_COMMENT];
 }
 
 /**
@@ -347,7 +372,7 @@ void Disasm_SetColumns(int *pos)
  * output is new column positions/values in 'newcols' array.
  * It's safe to use same array for both.
  */
-void Disasm_DisableColumn(int column, int *oldcols, int *newcols)
+void Disasm_DisableColumn(int column, const int *oldcols, int *newcols)
 {
 	int i, diff = 0;
 
@@ -381,11 +406,48 @@ int Disasm_GetOptions(void)
 }
 
 /**
- * Set CPU and FPU mask used for disassembly (when changed from the UI or the options)
+ * Initialize disassembly options from config
  */
-void Disasm_SetCPUType(int CPU, int FPU, bool bMMU)
+void Disasm_Init(void)
 {
-	switch (CPU)
+	options = ConfigureParams.Debugger.nDisasmOptions;
+	if (ConfigureParams.Debugger.bDisasmUAE)
+	{
+		if (options & doptOpcodesSmall)
+			disasm_flags |= (DISASM_FLAG_LC_MNEMO | DISASM_FLAG_LC_SIZE);
+		else
+			disasm_flags &= ~(DISASM_FLAG_LC_MNEMO | DISASM_FLAG_LC_SIZE);
+
+		if (options & doptRegisterSmall)
+			disasm_flags |= DISASM_FLAG_LC_REG;
+		else
+			disasm_flags &= ~DISASM_FLAG_LC_REG;
+
+		if (options & doptNoWords)
+			disasm_flags &= ~DISASM_FLAG_WORDS;
+		else
+			disasm_flags |= DISASM_FLAG_WORDS;
+
+		if (options & doptShowValues)
+			disasm_flags |= (DISASM_FLAG_CC | DISASM_FLAG_EA | DISASM_FLAG_VAL);
+		else
+			disasm_flags &= ~(DISASM_FLAG_CC | DISASM_FLAG_EA | DISASM_FLAG_VAL);
+
+		if (options & doptHexSmall)
+			disasm_flags |= DISASM_FLAG_LC_HEX;
+		else
+			disasm_flags &= ~DISASM_FLAG_LC_HEX;
+
+		disasm_init();
+		return;
+	}
+	/* ext disassembler */
+	if (options & doptNoWords)
+		Disasm_DisableColumn(DISASM_COLUMN_HEXDUMP, defaultPositions, positions);
+	else
+		memcpy(positions, defaultPositions, sizeof(positions));
+
+	switch (ConfigureParams.System.nCpuLevel)
 	{
 #ifdef HAVE_CAPSTONE_M68K
 	 case 0:  optionCPUTypeMask = CS_MODE_M68K_000; break;
@@ -411,10 +473,13 @@ const char *Disasm_ParseOption(const char *arg)
 			int flag;
 			const char *desc;
 		} option[] = {
-			{ doptNoSpace, "no space after comma in the operands list" },
-			{ doptOpcodesSmall, "opcodes in small letters" },
-			{ doptRegisterSmall, "register names in small letters" },
-			{ doptStackSP, "stack pointer as 'SP', not 'A7'" },
+			{ doptNoSpace, "ext: no space after comma in the operands list" },
+			{ doptOpcodesSmall, "opcodes in lower case" },
+			{ doptRegisterSmall, "register names in lower case" },
+			{ doptStackSP, "ext: stack pointer as 'SP', not 'A7'" },
+			{ doptNoWords, "do not show 16-bit words at this address" },
+			{ doptShowValues, "uae: show EA + CC values after instruction" },
+			{ doptHexSmall, "uae: hex numbers in lower case" },
 			{ 0, NULL }
 		};
 		int i;
@@ -427,7 +492,7 @@ const char *Disasm_ParseOption(const char *arg)
 		      "Flag values:\n", stderr);
 		for (i = 0; option[i].desc; i++) {
 			assert(option[i].flag == (1 << i));
-			fprintf(stderr, "\t%d: %s\n", option[i].flag, option[i].desc);
+			fprintf(stderr, "\t0x%02x: %s\n", option[i].flag, option[i].desc);
 		}
 		fprintf(stderr, "Current settings are:\n\t--disasm %s --disasm 0x%x\n",
 			ConfigureParams.Debugger.bDisasmUAE ? "uae" : "ext",
@@ -437,16 +502,18 @@ const char *Disasm_ParseOption(const char *arg)
 	if (strcasecmp(arg, "uae") == 0)
 	{
 		fputs("Selected UAE CPU core internal disassembler.\n", stderr);
+		fprintf(stderr, "Disassembly output flags are 0x%x.\n", options);
 		ConfigureParams.Debugger.bDisasmUAE = true;
-		disasm_init();
+		Disasm_Init();
 		return NULL;
 	}
 	if (strcasecmp(arg, "ext") == 0)
 	{
 #if HAVE_CAPSTONE_M68K
 		fputs("Selected external disassembler.\n", stderr);
-		fprintf(stderr, "Disassembly output flags are %d.\n", options);
+		fprintf(stderr, "Disassembly output flags are 0x%x.\n", options);
 		ConfigureParams.Debugger.bDisasmUAE = false;
+		Disasm_Init();
 		return NULL;
 #else
 		return "external disassembler (capstone) not compiled into this binary";
@@ -455,18 +522,24 @@ const char *Disasm_ParseOption(const char *arg)
 	if (isdigit((unsigned char)*arg))
 	{
 		char *end;
-		int newopt = strtol(arg, &end, 0);
+		unsigned int newopt = strtol(arg, &end, 0);
 		if (*end)
 		{
 			return "not a number";
 		}
-		if ((newopt|optionsMask) != optionsMask)
+		if ((newopt|extOptMask|uaeOptMask) != (extOptMask|uaeOptMask))
 		{
 			return "unknown flags in the bitmask";
 		}
-		fprintf(stderr, "Changed CPU disassembly output flags from %d to %d.\n", options, newopt);
-		ConfigureParams.Debugger.nDisasmOptions = options = newopt;
-		Disasm_CheckOptionEngine();
+		if (newopt != options)
+		{
+			fprintf(stderr, "Changed CPU disassembly output flags from 0x%x to 0x%x.\n", options, newopt);
+			ConfigureParams.Debugger.nDisasmOptions = options = newopt;
+			Disasm_CheckOptionEngine(options);
+			Disasm_Init();
+		}
+		else
+			fprintf(stderr, "No CPU disassembly options changed.\n");
 		return NULL;
 	}
 	return "invalid disasm option";
diff --git a/src/debug/68kDisass.h b/src/debug/68kDisass.h
index 60fb4960..ba9a9c39 100644
--- a/src/debug/68kDisass.h
+++ b/src/debug/68kDisass.h
@@ -24,10 +24,10 @@ enum {
 
 extern void Disasm_GetColumns(int *columns);
 extern void Disasm_SetColumns(int *columns);
-extern void Disasm_DisableColumn(int column, int *oldcols, int *newcols);
+extern void Disasm_DisableColumn(int column, const int *oldcols, int *newcols);
 
 extern const char* Disasm_ParseOption(const char *arg);
 extern int Disasm_GetOptions(void);
-void Disasm_SetCPUType(int CPU ,int FPU, bool bMMU);
+void Disasm_Init(void);
 
 #endif		/* HATARI_68KDISASS_H */
diff --git a/tests/debugger/test-dummies.c b/tests/debugger/test-dummies.c
index 16e6966e..17684ab2 100644
--- a/tests/debugger/test-dummies.c
+++ b/tests/debugger/test-dummies.c
@@ -206,4 +206,4 @@ uint32_t Disasm_GetNextPC(uint32_t pc) { return pc+2; }
 void Disasm (FILE *f, uaecptr addr, uaecptr *nextpc, int count) {}
 void Disasm_GetColumns(int *columns) {}
 void Disasm_SetColumns(int *columns) {}
-void Disasm_DisableColumn(int column, int *oldcols, int *newcols) {}
+void Disasm_DisableColumn(int column, const int *oldcols, int *newcols) {}
-- 
2.30.2



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/