Re: [COMMITTERS] packing/alignment annotation for ItemPointerData redux

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Greg Stark <stark(at)mit(dot)edu>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: [COMMITTERS] packing/alignment annotation for ItemPointerData redux
Date: 2016-10-19 16:20:54
Message-ID: 12448.1476894054@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

[ moved to -hackers ]

Greg Stark <stark(at)mit(dot)edu> writes:
> 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.

Ick. Can you look to see how those macros are expanding on your clang?

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

I've never seen this on MacOS. The current compiler version (on Sierra)
is

$ gcc -v
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/c++/4.2.1
Apple LLVM version 8.0.0 (clang-800.0.38)
Target: x86_64-apple-darwin16.0.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Don't know how that version number compares to "3.8.0".

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Greg Stark 2016-10-19 16:30:03 Re: [COMMITTERS] packing/alignment annotation for ItemPointerData redux
Previous Message Greg Stark 2016-10-19 15:50:17 packing/alignment annotation for ItemPointerData redux

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2016-10-19 16:29:31 Re: Move pg_largeobject to a different tablespace *without* turning on system_table_mods.
Previous Message Kevin Grittner 2016-10-19 16:15:38 Re: Question about behavior of snapshot too old feature