[hatari-devel] Hatari crashes with Mental Hangover demo

Nicolas Pomarède npomarede at corp.free.fr
Sat Apr 30 23:33:24 CEST 2011


Le 30/04/2011 22:09, Eero Tamminen a écrit :
> Hi,
>
> On lauantai 30 huhtikuu 2011, Nicolas Pomarède wrote:
>> But in fact I think we should have some kind of "DMA_memcpy" that would
>> be used from FDC / HDC when we want to copy memory from Hatari to the
>> emulated RAM. This function would check each written byte is in ST RAM
>> or I/O space and would mask addresses to 24 bits.
>
> 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 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).


>
>> 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.
Unless we encounter bugs due to this, I'm not sure this is a top 
priority to handle this case.


>
>> In that case, ignoring is fine, because it's not the cause of the
>> problem. But as above, it would be more correct to not ignore the
>> request but ensure each destination address is corrrecly handled to stay
>> in the RAM / ROM / IO space of the emulated machine (even if writing in
>> ROM should have no effect, it should not be forbidden)
>
> The check in stMemory.h is:
> -------------
> static inline bool STMemory_ValidArea(Uint32 addr, int size)
> {
>          if (size>= 0&&  addr+size<  0xff0000&&
>              (addr+size<  STRamEnd || addr>= 0xe00000))
>          {
>                  return true;
>          }
>          return false;
> }
> -------------
>
> 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)

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 godd thing.

Nicolas



More information about the hatari-devel mailing list