mesa: Fix zeroing of new ParameterValues array entries when growing
authorKenneth Graunke <kenneth@whitecape.org>
Tue, 19 Sep 2023 00:43:01 +0000 (17:43 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Tue, 26 Sep 2023 06:26:41 +0000 (23:26 -0700)
commit1c1aa579ff5214a2082ad29ef99d76b52d76d560
treea10abefb91c752497717b68ff8ac3790c940a10f
parentf79617fe804ea6524651ff1bc3a91098d3199179
mesa: Fix zeroing of new ParameterValues array entries when growing

On non-Windows OSes, align_realloc is the os_realloc_aligned() from
src/util/os_memory_aligned.h, which doesn't use realloc internally.
Instead, it uses os_malloc_aligned() and memcpy's over the old data,
which is why it needs an "old size" (unlike normal realloc).

In _mesa_reserve_parameter_storage, the call to align_realloc above
passes (oldValNum * sizeof(gl_constant_value)) as the old size, which
is all the actual data.  The actual allocation size of the array may
be larger (in fact, we allocate 16 extra components), which is tracked
in SizeValues.  After realloc, we memset to zero starting at the old
allocation size, to the new allocation size.

This would work if it were a real realloc.  However, because we actually
malloc + memcpy and only copy the previous /data/, not the allocated
size, and then memset from the old /allocated size/, our new copy will
have the spaces between the old data and the old allocation size neither
copied nor memset, leaving them as uninitialized garbage memory.

These values then get written to the shader cache, meaning that if you
compile the same shader multiple times, you may get different shader
cache entries.  This is bad for reproducible, deterministic compiles.

While at it, we also memset to zero in _mesa_add_parameter, as this
looks like another place where memset-to-zero is missing.

To reproduce this error, one can run shader-db:

$ MESA_SHADER_CACHE_DIR=a ./run -b shaders/godot3.4/49-28.shader_test
$ MESA_SHADER_CACHE_DIR=b ./run -b shaders/godot3.4/49-28.shader_test

and see an occasional difference in the end of the ParameterValues
array, where there's a padding gap between the last two elements that
was never zero-initialized.

Thanks to Mark Janes for discovering this and tracking it down together!

Cc: mesa-stable
Reviewed-by: Mark Janes <markjanes@swizzler.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25316>
src/mesa/program/prog_parameter.c