[hatari-devel] A new disassembler in Hatari

Eero Tamminen eerot at users.berlios.de
Sun Nov 21 00:38:00 CET 2010


Hi,

On Saturday 20 November 2010, Nicolas Pomarède wrote:
> > thanks a lot for your work, this looks really nice !
> >
> > I will add this to Hatari, along with an option to choose between
> > current uae's disassembler and your new one.
> >
> > I have a few errors when compiling with current Hatari gcc's flags, but
> > this should be easy to fix.
> >
> > I will commit the resulting files once they compile so anyone can try
>
> I committed the required changes to integrate your work in Hatari.
>
> As you can see, I had to change a few lines to avoid errors/warnings
> when compiling with our flags (this may depend on th egcc version used
> too). Maybe you can backport them to your code in case you plan to use
> it in some other project.

I did some additional fixes.  E.g. protos for global functions don't belong
into *.c files (those functions were local i.e. they should be static).


> Remaining harmless warnings are :
>
> 68kDisass.c: In function 'Disass68kEA':
>
> 68kDisass.c:821:6: attention : suggest explicit braces to avoid
> ambiguous 'else'
> 68kDisass.c: In function 'Disass68k_loop':
> 68kDisass.c:2381:15: attention : array subscript is above array bounds
>
> At line 23811, I see your "extending" the string, which is ok since the
> buffer is 1024 bytes, but gcc is detecting the index was computed from
> strlen and gives a warning ; maybe there's a way to write this in a
> different way ?

I'm not getting that warning with my compiler (GCC 4.3.2).
What happens if you declare all the optionPos* variables as const?


> Regarding the integration, I replaced calls to m68k_disasm by Disasm
> with a new parameter DISASM_ENGINE_UAE or DISASM_ENGINE_EXT.

Uh, that's now set in multiple places in code.


> This parameter (as well as the other options available in your code)
> could of course be stored using some settings in hatari.cfg (Eero, if
> you fell like doing it ?)

Would somebody still want to use the UAE disassembler?

If yes, the option could be either a define set by CMake or be a Hatari
[Debugger] section configuration option.  How likely people would be
to change that?


> While doing this, I saw breakcond.c is using stderr instead of
> debugOutput ; Eero, is this normal ?

I think the purpose of the debugOutput is just to get output of (specific)
debugger commands.  What do you think?

 (it's currently used by Hatari UI debugger to get disassembly, registers or
memory hexdump and it's nice not to need to filter the other stuff out
of that...)


> Regarding the asm output, I really prefer this new presentation, "moveq"
> or "movem" are really easier to read for example.
>
> There're 2 cases I'm less used to :
>
> 1) adding paranthesises aroung absolute addr
> 	lea       ($ffff8907).w,a0
> 	move.l    d0,($2134e).l
>
> I think it's more common to just have :
> 	lea       $ffff8907.w,a0
> 	move.l    d0,$2134e.l
>
> I checked that devpac handled both notations, but could it be possible
> to not add paranthesises around absolute addresses ?

The parenthesis notion is one used by the debugger itself for
breakpoints, so I guess it would be more consistent. :-)


> 2) adding .l suffix after address
> 	jsr       ($1ec50).l
> 	move.w    d1,($212ec).l
>
> As most common case is to have 32 bit addr, could you remove the .l when
> the address is 32 bit, and just add a .w when the address is a short 16
> bit one ?

IMHO clearer to have those always.  Things align then similarly etc.


> 08b8 0001 820a         bclr      #1,$ffff820a.w  -> short addr mode
>
>
> What do you think ?
>
>
> PS : I didn't modify winuae's newcpu.c in cpu/ for now, we can change
> this cosmetic points later.

The new disassembler code's broken indenting needs to be fixed also...


	- Eero



More information about the hatari-devel mailing list