Re: PgStat_HashKey padding issue when passed by reference

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PgStat_HashKey padding issue when passed by reference
Date: 2025-09-06 01:35:58
Message-ID: aLuP_kb6WhHB7LxO@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-09-06 01:38:35 Re: [PATCH] Update parser README to include parse_jsontable.c
Previous Message Michael Paquier 2025-09-06 01:12:11 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible