Re: pg_verify_checksums and -fno-strict-aliasing
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Banck <michael.banck@credativ.de>
Cc: Andres Freund <andres@anarazel.de>, Magnus Hagander <magnus@hagander.net>,
Fabien COELHO <coelho@cri.ensmp.fr>,
pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-08-31T16:02:04Z
Lists: pgsql-hackers
Michael Banck <michael.banck@credativ.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
Commits
-
Avoid using potentially-under-aligned page buffers.
- 44cac9346479 12.0 landed
- 5af055ed7446 9.3.25 landed
- f5c93cf92223 11.0 landed
- 826980424538 9.6.11 landed
- 10b9af3ebbed 10.6 landed
- 083d9ced14e4 9.4.20 landed
- 03ffe5553c1d 9.5.15 landed
-
Code review for pg_verify_checksums.c.
- d787af7badfe 11.0 landed
- d9c366f9e801 12.0 landed
-
Make checksum_impl.h safe to compile with -fstrict-aliasing.
- db87d3b5253f 9.3.25 landed
- 20f9cd55dd53 9.4.20 landed
- 853af991e35a 9.5.15 landed
- d6ef17ed7bba 9.6.11 landed
- c2dfbd18ce25 10.6 landed
- 9daff2fe69dc 11.0 landed
- 8c62d9d16f01 12.0 landed