From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Thomas Munro <thomas.munro@enterprisedb.com>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: Ensuring hash tuples are properly maxaligned
In-reply-to: <20180103010931.6vsirlb5pdapdroq@alap3.anarazel.de>
References: <14483.1514938129@sss.pgh.pa.us>
 <20180103010931.6vsirlb5pdapdroq@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Tue, 02 Jan 2018 17:09:31 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <17453.1514942419.1@sss.pgh.pa.us>
Date: Tue, 02 Jan 2018 20:20:19 -0500
Message-ID: <17454.1514942419@sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> On 2018-01-02 19:08:49 -0500, Tom Lane wrote:
>> Now, the existing definition of the struct seems safe on all
>> architectures we support, but it would not take much to break it.
>> I think we ought to do what we did recently in the memory-context
>> code: insert an explicit padding calculation and a static assertion
>> that it's right.  Hence, the attached proposed patch (in which
>> I also failed to resist the temptation to make the two code paths
>> in dense_alloc() look more alike).  Any objections?

> But note that dsa_pointer can be wider than a regular pointer on
> platforms without atomics support.

Hm.  I did not get that impression from the comments in dsa.h,
but if it's true then this approach won't work --- and indeed the
hash code would be actively broken in such a case, so it's a problem
we must fix.

The other idea I was considering was to get rid of the data[] member
entirely, define HASH_CHUNK_HEADER_SIZE as
MAXALIGN(sizeof(HashMemoryChunkData)), and replace the references to
data[] with pointer arithmetic along the lines of
	((char *) chunk) + HASH_CHUNK_HEADER_SIZE
This would be a bit more invasive, but probably not too ugly if we
encapsulated that pointer arithmetic in a macro.  And it'd certainly
be a bunch more robust against people throwing random fields into
the struct.

			regards, tom lane