On Thu, Sep 04, 2025 at 11:50:15AM -0500, Sami Imseih wrote: > Perhaps calling this a compiler bug is not appropriate. > However, memset is in fact called when the key is created > > ``` > /* clear padding */ > memset(&key, 0, sizeof(struct PgStat_HashKey)); > ``` > > but the zeroed out padding bytes are not retained when the > key is passed by value. This is why the proposal is to pass the > key by reference.
So your point is that this impacts the hash lookups and the fact that we rely on passing PgStat_HashKey around due to the internals of how pgstat_shmem.c is shaped for its dshash. Did you run a full installcheck under valgrind to have more confidence that this is the only code path where we care about the padding of the passed-in values for the hash lookups? There are a lot of things that use simplehash.h, so one could question if we should think about that from a higher point of view, and if there could be some compiler magic that could be used to enforce a policy (don't think there is any magic for the padding of passed-in values, just wondering if there is something I don't know about that could justify a more global policy that can be applied to everything that uses simplehash.h). One idea would be, for example, that keys used with simplehash should never have any padding at all, and we could force a check in the shape of a static assertion to force a failure when attempting to compile code that attempts to do so. That would give us a way to check in a broader way if some code path do that currently, scaling better with the expectations we could have in the whole tree or even out-of-core extension code. -- Michael
signature.asc
Description: PGP signature
