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>
Subject: Re: pg_verify_checksums and -fno-strict-aliasing
In-reply-to: <1535703067.1286.10.camel@credativ.de>
References: <1535618100.1286.3.camel@credativ.de>
 <alpine.DEB.2.21.1808301325070.32764@lancre> <7321.1535639966@sss.pgh.pa.us>
 <CABUevEz+3g4-tKLgcjTNK7EwgHm95NSYSKTNyEjcnQEGRzie0A@mail.gmail.com>
 <1535660901.1286.8.camel@credativ.de> <25968.1535663968@sss.pgh.pa.us>
 <20180830214606.xpm4bho5nmhfuzlx@alap3.anarazel.de>
 <20180830220409.4zhdka6ibs66enbk@alap3.anarazel.de>
 <28544.1535667100@sss.pgh.pa.us>
 <20180830223315.m2lc57oay3kztjz4@alap3.anarazel.de>
 <7502.1535670135@sss.pgh.pa.us> <1535703067.1286.10.camel@credativ.de>
Comments: In-reply-to Michael Banck <michael.banck@credativ.de>
	message dated "Fri, 31 Aug 2018 10:11:07 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <365.1535731324.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 31 Aug 2018 12:02:04 -0400
Message-ID: <366.1535731324@sss.pgh.pa.us>
Precedence: bulk

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-stat=
ement -Werror=3Dvla -Wendif-labels -Wmissing-format-attribute -Wformat-sec=
urity -fwrapv -fexcess-precision=3Dstandard -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 wi=
ll break strict-aliasing rules [-Wstrict-aliasing]
   if (PageIsNew(buf))
   ^~

Adding -Wstrict-aliasing=3D2 makes it slightly more verbose:

$ gcc -Wstrict-aliasing=3D2 -Wall -Wmissing-prototypes -Wpointer-arith -Wd=
eclaration-after-statement -Werror=3Dvla -Wendif-labels -Wmissing-format-a=
ttribute -Wformat-security -fwrapv -fexcess-precision=3Dstandard -g -O2 -I=
../../../src/include  -D_GNU_SOURCE   -c -o pg_verify_checksums.o pg_verif=
y_checksums.c
pg_verify_checksums.c: In function 'scan_file':
pg_verify_checksums.c:82:2: warning: dereferencing type-punned pointer wil=
l break strict-aliasing rules [-Wstrict-aliasing]
  PageHeader header =3D (PageHeader) buf;
  ^~~~~~~~~~
pg_verify_checksums.c:112:3: warning: dereferencing type-punned pointer wi=
ll 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