Re: Ensuring hash tuples are properly maxaligned
Tom Lane <tgl@sss.pgh.pa.us>
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
Date: 2018-01-03T01:20:19Z
Lists: pgsql-hackers
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
Commits
-
Ensure proper alignment of tuples in HashMemoryChunkData buffers.
- 5dc692f78d3b 11.0 landed