From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>,
        Thomas Munro <thomas.munro@gmail.com>
Subject: Re: Pointer subtraction with a null pointer
In-reply-to: <51678.1648312664@sss.pgh.pa.us>
References: <48826.1648310694@sss.pgh.pa.us> <49360.1648311192@sss.pgh.pa.us>
 <20220326163441.qzpcioycozbzarsf@alap3.anarazel.de>
 <51678.1648312664@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sat, 26 Mar 2022 12:37:44 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <55127.1648315414.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sat, 26 Mar 2022 13:23:34 -0400
Message-ID: <55128.1648315414@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/55128.1648315414%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2022-03-26 12:13:12 -0400, Tom Lane wrote:
>>> This code is old, but mylodon wasn't doing that a week ago, so
>>> Andres must've updated the compiler and/or changed its options.

>> Yep, updated it to clang 13. It's a warning present in 13, but not in 1=
2.

> OK, that answers that.

... Actually, after looking closer, I misread what our code is doing.
These call sites are trying to set the relptr value to "null" (zero),
and AFAICS it should be allowed:

freepage.c:188:2: warning: performing pointer subtraction with a null poin=
ter has undefined behavior [-Wnull-pointer-subtraction]
        relptr_store(base, fpm->btree_root, (FreePageBtree *) NULL);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../src/include/utils/relptr.h:63:59: note: expanded from macro 'r=
elptr_store'
         (rp).relptr_off =3D ((val) =3D=3D NULL ? 0 : ((char *) (val)) - (=
base)))
                                                ~~~~~~~~~~~~~~~~ ^

clang is complaining about the subtraction despite it being inside
a conditional arm that cannot be reached when val is null.  It's hard
to see how that isn't a flat-out compiler bug.

However, granting that it isn't going to get fixed right away,
we could replace these call sites with "relptr_store_null()",
and maybe get rid of the conditional in relptr_store().

			regards, tom lane