Re: [hatari-devel] Uninitialized value errors from Valgrind

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On 2.9.2023 10.58, Nicolas Pomarède wrote:
> Le 02/09/2023 à 00:39, Eero Tamminen a écrit :
> > These are from running ScummVM in emulated 32Mhz Falcon + FPU with 
> > 8+64MB RAM, and DSP disabled, using latest EmuTOS git version.
> >
> > EmuTOS boot:
> > ==5238== Conditional jump or move depends on uninitialised value(s)
> > ==5238==    at 0x7FC8C9: op_6601_23_ff (cpuemu_23.c:28838)
> > ==5238==    by 0x340AF0: m68k_run_2ce (newcpu.c:7036)
> > ==5238==    by 0x33CDE1: m68k_go (newcpu.c:7832)
> > ==5238==    by 0x2B5576: main (main.c:983)
> >
> > Corresponds to generated code:
> >          /* OP zero */
> >          if (cctrue(6)) {
>
> due to some optimisations in cpu core, valgrind or other tools are 
> sometimes reporting false positive.
> Here for example it's unlikely that regflags.cznv (which is used by 
> cctrue() ) is non initialised. Or maybe valgrind points to line 28838 
> but it could be anything inside the 'if' ?
>
> Perhaps compiling hatari without -O flags could avoid more optimisations 
> and help valgrind give a more precise cause on the uninitialised value ?

Adding "--track-origins=yes" option to Valgrind, shows following:
---------------------------------
==4142==  Uninitialised value was created by a heap allocation
==4142==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
==4142==    by 0x35F1A1: memory_init (memory.c:1761)
==4142==    by 0x30068F: TOS_InitImage (tos.c:1132)
==4142==    by 0x2F1FA8: Reset_ST (reset.c:61)
==4142==    by 0x2B53A6: Main_Init (main.c:769)
==4142==    by 0x2B53A6: main (main.c:965)
---------------------------------

Which is actually allocation of TT-RAM.

I think Valgrind is doing a bit more backtracing on where values come 
from. I.e. it notices that regflags is set based on value fetched from 
TT-RAM.

Looking at Hatari memory.c, there's no clearing of TT-RAM or ST-RAM, as 
_"memory_clear()" functionality is ifdeffed out_.

I do not think TOS is clearing RAM at boot either, nor doing it for any 
of the Atari application allocations, nor their stack area.


> > Then further issues from CPU core while ScummVM startup continued:
> > ==5238== Conditional jump or move depends on uninitialised value(s)
> > ==5238==    at 0xC1E2F4: m68k_mull (newcpu_common.c:1357)
> > ==5238==    by 0x7F287B: op_4c3c_23_ff (cpuemu_23.c:21610)
> > ==5238==    by 0x340AF0: m68k_run_2ce (newcpu.c:7036)
> > ==5238==    by 0x33CDE1: m68k_go (newcpu.c:7832)
> > ==5238==    by 0x2B5576: main (main.c:983)
>
> here also I don't see the cause. I guess the possible variable is "a", 
> but it can't be uninitialised as we have a few lines above "a *= 
> (uae_s64)(uae_s32)src"

Assignments for "a" in in "m68k_mull(opcode, src, extra)":
  uae_s64 a = (uae_s64)(uae_s32)m68k_dreg (regs, (extra >> 12) & 7);
  a *= (uae_s64)(uae_s32)src;

rely on values prefetched from memory:
  uae_s16 extra = get_word_ce030_prefetch(2);
  uae_s32 dst = get_long_ce030_prefetch(4)


There were some backtraces which did not list origin, but as there were 
those same backtraces also with origins pointing to Hatari ST-/TT-RAM 
malloc, I assume that's the case with all of them.

(E.g. for forscreenConvert.c, every other backtrace included origin, and 
rest did not.)


	- Eero