Re: [hatari-devel] ASAN issues with tests

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sat, 30 Jul 2022 06:08:48 +0000
schrieb Thomas Huth <th.huth@xxxxxxxxx>:

> Am Fri, 29 Jul 2022 23:40:56 +0300
> schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> 
> > Hi,
> > 
> > On 29.7.2022 18.26, Eero Tamminen wrote:  
> > > It does not give error if VDI screen size is small enough.
> > > 
> > > And it does not happen with Videl code (used for Falcon & TT).    
> > 
> > Test tries to set screen to the end of emulated machine RAM.
> > 
> > I'm not able to reproduce ASAN error if I disable SMALL_MEM build 
> > option, i.e. it's a SMALL_MEM config bug.
> > 
> >   
> > > With "--vdi-planes 4", 640x200 is enough to trigger it, with 2 planes 
> > > 640x400, and with mono, 640x736.    
> > 
> > All these cause screen area to be slightly larger
> > than the 56 KB padding Thomas added in his SMALL_MEM workaround:
> > https://git.tuxfamily.org/hatari/hatari.git/commit/?id=5316b4081e9d3fa177ed14419369bc13892af4e8
> > 
> > It appears that SMALL_MEM ST-RAM padding should take into account also 
> > largest possible VDI screen size (300KB), i.e. use MAX_VDI_BYTES.  
> 
> Theoretically, this should not be necessary. The 56k "runaway ramp" is only
> there for the ST/STE screen mode rendering functions from screen.c. The
> generic functions from screenConvert.c should contain proper checks a la:
> 
> 		if (nLineEndAddr > STRamEnd)
> 		{
> 			Screen_memset_uint32(hvram_line, palette.native[0], pitch);
> 			hvram_line += pitch;
> 			continue;
> 		}
> 
> I wonder why they don't apply here. I'll try to have a look at this today.

Ok, I think I've got it now: The problem was that the ST program could have
changed the video base address again between the point in time we recorded
the VideoBase variable and the point in time the rendering was done in
screenConvert.c. So the code in screenConvert.c must not use
Video_GetScreenBaseAddr() but has to use the vide base address value that
was recorded earlier. I've committed a patch - that will hopefully fix this
issue.

 Thomas