Re: pg_verify_checksums and -fno-strict-aliasing

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Banck <michael(dot)banck(at)credativ(dot)de>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Magnus Hagander <magnus(at)hagander(dot)net>, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_verify_checksums and -fno-strict-aliasing
Date: 2018-08-31 16:02:04
Message-ID: 366.1535731324@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael Banck <michael(dot)banck(at)credativ(dot)de> writes:
> Am Donnerstag, den 30.08.2018, 19:02 -0400 schrieb Tom Lane:
>> Maybe. In any case, the attached version avoids any need for memcpy
>> and is, I think, cleaner code anyhow. It fixes the problem for me
>> with Fedora's gcc, though I'm not sure that it'd be enough to get rid
>> of the warning Michael sees (I wonder what compiler he's using).

> This fixes the bogus checksums, thanks!
> I am on Debian stable, i.e. 'gcc version 6.3.0 20170516 (Debian 6.3.0-
> 18+deb9u1)'.

Ah-hah. I still can't replicate any warning with gcc 8.1.1, but
I do have a Raspbian installation at hand, with
gcc version 6.3.0 20170516 (Raspbian 6.3.0-18+rpi1+deb9u1)
and on that I get

$ gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fwrapv -fexcess-precision=standard -g -O2 -I../../../src/include -D_GNU_SOURCE -c -o pg_verify_checksums.o pg_verify_checksums.c
pg_verify_checksums.c: In function 'scan_file':
pg_verify_checksums.c:112:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (PageIsNew(buf))
^~

Adding -Wstrict-aliasing=2 makes it slightly more verbose:

$ gcc -Wstrict-aliasing=2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fwrapv -fexcess-precision=standard -g -O2 -I../../../src/include -D_GNU_SOURCE -c -o pg_verify_checksums.o pg_verify_checksums.c
pg_verify_checksums.c: In function 'scan_file':
pg_verify_checksums.c:82:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
PageHeader header = (PageHeader) buf;
^~~~~~~~~~
pg_verify_checksums.c:112:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (PageIsNew(buf))
^~

Compiling this way also causes pg_verify_checksums to compute wrong
checksums. Installing the patched version of checksum_impl.h fixes
that, but doesn't change the warnings, meaning they have absolutely
nothing to do with the actual problem :-(

So, even aside from the difficulty of getting rid of aliasing-unsafe
code in Postgres, the state of the compiler warning technology is
still years short of where we could sanely consider dispensing with
-fno-strict-aliasing in general. Still, as I said before, it'd be
a good idea for checksum_impl.h to not depend on that. I'll go
ahead and push the fix.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Georgy Buranov 2018-08-31 16:30:28 Re: PostgreSQL logical decoder output plugin - unchanged toast data
Previous Message Andres Freund 2018-08-31 15:39:12 Re: PostgreSQL logical decoder output plugin - unchanged toast data