[hatari-devel] Hatari crashes with Mental Hangover demo

[Eero Tamminen]

Hi,

On sunnuntai 01 toukokuu 2011, Nicolas Pomarède wrote:
> Le 30/04/2011 22:09, Eero Tamminen a écrit :
>> On lauantai 30 huhtikuu 2011, Nicolas Pomarède wrote:
>> I added stMemory_SafeCopy() function.
> 
> OK for this function, but I'm not sure you should return false when
> memory is not completly valid ; on a real ST, I think the DMA writes
> data to the dest pointer wherever it points and always successes (except
> that if you write to ROM for example, the dma writes would have no
> effect).

I don't think the copy operation should set a policy on it, that's up to
the caller.  Caller can igore the return value (as I didn't add a GCC
attribute that would give warning if it isn't).

 
> I think it would be better to always return true and don't set return
> code to HD_STATUS_ERROR in hdc.c  (this would need to be tested on real
> hardware, but I don't recall I already saw cases where dma failed
> because of the dest address).

In the HDC case it's more of a HD command failing to do what was requested
than DMA failure.  I didn't check the hdc.c code to see whether it's
actually propagated to the Atari side though.

I would appreciate somebody with more knowledge about the HD side commenting 
on this...  Thomas?  Uwe?


> >> This way, we would have a common function to correctly copy data from
> >> disk images to the emulated RAM or IO space.
> > 
> > Half of the potentially unsafe operations are fread()s&  fwrite()s to
> > STRam, not just memcpy()s...  See e.g. gemdos.c which I've fixed
> > earlier or the new hdc.c changes.
> 
> Yes, to be correct we would need some similar checks to ensure we wrap
> to the start of the RAM once we reach the end of the RAM when
> reading/writing.

Those do have checks, but no wrapping.


> Unless we encounter bugs due to this, I'm not sure this is a top
> priority to handle this case.

I don't know any case e.g. for gemdos emulation where it would be something
that would need to be emulated instead of just a bug in the emulated program
which Hatari must guard against to be robust.


> > stMemory_SafeCopy() copies only bytes that above states to be valid, it
> > ignores bytes going to invalid addresses. (As I don't see what it e.g.
> > should do to bytes between STRamEnd and 0xe00000.)
> 
> I agree for byte between end of ram and IO space, but you should take
> into account the fact that the RAM is "mirrored" when you only have 0.5
> MB or 1 MB out of 4 MB.
> 
> I think the function should "wrap" when addr > STRamEnd, not just ignore
> the write.
>
> For example, on a 512 KB ST, if DMA writes $1000 bytes to addr $7ff00,
> it will reach up to $81f00, which in fact becomes addresses 0 to $1f00.
> So in the end, the DMA really write to $7ff00-$7ffff and to 0-$1eff
> 
> If addr < 4 MB, addr should become addr % CURRENT_RAM_SIZE ; above 4 MB,
> write should be ignored (this might be different on ST (TT ? Falcon ?
> Mega ST ?) with up to 16 MB of RAM)

Feel free to improve the SafeCopy function. :-)

 
> Well, anyway I'm not sure any program is relying on this to willingly
> write data with DMA on a boundary of the RAM.
> At least stMemory_SafeCopy will prevent crash in Hatari itself which is
> already a good thing.

Yes, that and logging that the emulated program has an issue are the main
things.


	- Eero