packing/alignment annotation for ItemPointerData redux

From: Greg Stark <stark(at)mit(dot)edu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: packing/alignment annotation for ItemPointerData redux
Date: 2016-10-19 15:50:17
Message-ID: CAM-w4HPJjEt7jjX=mCY5mC1CYEB-b7NTySfyc=51i3-Kh5F2Ew@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Back in 2001 a hack to add __attribute__((packed)) to ItemPtr was
added with a comment "Appropriate whack upside the head for ARM"
(dcbbdb1b3ee). I don't know if this is still a factor in 2016 or not
but it has already resulted in some collateral damage in 2015 when
some compiler took that as license to align the whole struct on single
byte alignment when it was buried inside another struct
(d4b538ea367de).

I just tried compiling with Clang 3.8.0 and got tons of warnings about
this because:

'ItemPointerData' may result in an unaligned pointer value
[-Waddress-of-packed-member]
...ItemPointerGetBlockNumber(&(xlrec->target_tid)),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../src/include/storage/itemptr.h:69:25: note: expanded from macro
'ItemPointerGetBlockNumber'
BlockIdGetBlockNumber(&(pointer)->ip_blkid) \
~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
../../../src/include/storage/block.h:118:19: note: expanded from macro
'BlockIdGetBlockNumber'
(BlockNumber) (((blockId)->bi_hi << 16) | ((uint16) (blockId)->bi_lo)) \
^~~~~~~

Which seems to indicate that clang may not understand the
"pg_attribute_aligned(2)" or perhaps it does and just doesn't take it
into account when generating these warnings.

I'm sure there are other people testing clang -- isn't it the default
on MacOS? Do they not see these warnings?

--
greg

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-10-19 16:20:54 Re: [COMMITTERS] packing/alignment annotation for ItemPointerData redux
Previous Message Peter Eisentraut 2016-10-19 13:58:46 pgsql: Make getrusage() output a little more readable

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2016-10-19 15:53:56 Re: Parallel bitmap heap scan
Previous Message Robert Haas 2016-10-19 15:47:25 Re: Transactions involving multiple postgres foreign servers