Re: [hatari-devel] Regression in Hatari w.r.t. "ST High" video mode

[Thomas Huth <th.huth@xxxxxxxxx>]

On 14/06/2022 13.05, Christian Zietz wrote:
> Hello,
>
> lately, I compiled Hatari for Linux on AArch64, which I haven't done
> before. There, I noticed that ST High (monochrome) video mode would not
> work. The screen would just stay white except for the status bar; while
> the floppy access indicator showed that otherwise the emulated ST was in
> fact running. On the same system, ST High works when compiling Hatari
> 2.3.1 instead.
> > Bisecting lead me to the following commit as cause for the regression:
> "Add sanity checks to avoid accesses beyond the end of the ST-RAM":
> https://git.tuxfamily.org/hatari/hatari.git/commit/?id=c3875fc8bf3148c376262a71966fd4bbcd54ff22. 
>
> I added some debug printfs to see the variables which are involved in
> the check that was added by this commit:
> fvram, nextline, (Uint16 *)&STRam[STRamEnd]
> DEBUG: 0x7f94b8e010 28 0x7f87c70010
> DEBUG: 0x7f94b01010 28 0x7f87c70010
>
> As you can see, the "fvram" is outside the ST RAM area, causing the
> check to fail and, therefore, the screen to be drawn white. I did not
> have time to fully debug this. But I think this happens because for mono
> mode a separately allocated screen buffer pSTScreen is used:
>
> https://git.tuxfamily.org/hatari/hatari.git/tree/src/screen.c#n735
> https://git.tuxfamily.org/hatari/hatari.git/tree/src/screen.c#n190
>
> If that memory allocation by chance happens to be *below* the ST RAM,
> the check passes; otherwise (like on my AAarch64 system), it fails.
> Imho, this needs to be fixed before a 2.4 release.

Oh, drat, I added the checks with src/falcon/videl.c in mind that calls 
Screen_GenConvert(&STRam[videoBase], ...), i.e. fvram indeed points 
somewhere in the STRam[] array here. But Screen_ConvertHighRes() from 
screen.c uses the buffer that you spotted instead, so these checks are 
currently wrong. Thanks a lot for the hint, Christian, I'll try to come up 
with a solution (might take a while, though, since I'm pretty busy this week).
As a temporary workaround, you could revert commit c3875fc8bf3148c and 
configure the sources with "--disable-small-mem", I think.

 Thomas