Re: [hatari-devel] Bug: cart_asm.s Pexec() implementation overwrites basepage command line end

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On torstai 08 marraskuu 2012, Eero Tamminen wrote:
> Here's a Hatari bug from Frank Wille (VBCC 68k backend maintainer)
> which I debugged...
> 
> In short, under GEMDOS HD emulation, Pexec() will overwrite
> 28 last bytes from the basebage with the loaded binary.
> If program command line is >99 characters long, last
> part of it will be garbage under GEMDOS HD emulation.
> 
> Problem happens also with Hatari v1.4, so it's pretty old.
> 
> I suspect that there's some bug in the cart_asm.s Pexec() wrapper.
> Thomas, as you've written that, could you take a look at this?

Looking at the cart_asm.s which corresponds to:
---
GEMDOS 0x4B Pexec(3, "SYSTEM.TTP", [119]"-bataritos ... hello.tos", 0x0)
GEMDOS 0x2F Fgetdta()
GEMDOS 0x4E Fsfirst("SYSTEM.TTP", 0x17)
GEMDOS 0x4F Fsnext()
GEMDOS 0x4B Pexec(5, 0x0, 0x1990e, 0x0)
GEMDOS 0x3D Fopen("SYSTEM.TTP", read-only)
-> FD 0 (read-only)
GEMDOS 0x3F Fread(64, 28, 0x1b15c)
GEMDOS 0x3F Fread(64, 2147483647, 0x1b178)
GEMDOS 0x3E Fclose(64)
---

The problem is pretty obvious (comments marked with "<-"):
---
[...]
        bsr.s   find_prog
        bsr.s   pexec5
        bsr.s   reloc
pexec5:
        move.l  10(a6),-(sp)
        move.l  6(a6),-(sp)
        clr.l   -(sp)
        move    #5,-(sp)
        move    #$4b,-(sp)      ; Pexec
        trap    #1              ; Gemdos
        lea             16(sp),sp
        tst.l   d0
        bmi.s   pexecerr
        rts
pexecerr:
        addq    #4,sp
        bra.s   gohome

reloc:
        movem.l a3-a5/d6-d7,-(sp)
        move.l  d0,a5           <- Pexec(5) returned basepage addr
        clr     -(sp)
        move.l  2(a6),-(sp)
        move    #$3d,-(sp)      ; Fopen
        trap    #1              ; Gemdos
        addq    #8,sp

        move.l  d0,d6
        move.l  a5,-(sp)        <- basepage address
        add.l   #228,(sp)       <- *adds 228 to it, not e.g. 256*
        pea     $1c.w           <- EXE header size before TEXT segment
        move    d6,-(sp)        <- file handle
        move    #$3f,-(sp)      ; Fread
        trap    #1              ; Gemdos  <- ...overwrites basepage
        lea     12(sp),sp

        cmp.w   #$601a,228(a5)  ; Check program header magic
        beq.s   magic_ok
[...]
magic_ok:
        ; TODO: check size!!
        move.l  a5,-(sp)        <- basepage again
        add.l   #256,(sp)       <- needs also to be fixed?
        pea             $7fffffff  <- "-1" i.e. all
        move    d6,-(sp)
        move    #$3f,-(sp)      ; Fread
        trap    #1              ; Gemdos
        lea             12(sp),sp


I think simplest would be just to add $1C to both of these offsets
and the offsets used by the code setting up the bss/data/text values
later on to the basepage.

However, real TOS seems to be putting text section right after
basepage like the above code does (to save 28 bytes of RAM I guess),
so probably for compatibility cart code should do the same.  E.g.
load the header to basepage+256, but copy the values to safety
(regs or vars) before reading rest of the file to the same position.

Thomas?


	- Eero