pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: printf %s with NULL pointer (was Re: BUG #17098: Assert failed on composing an error message when adding a type to an extension being dropped)
Date: Tue, 13 Jul 2021 10:29:28 -0400
Message-ID: <3227956.1626186568@sss.pgh.pa.us> (raw)
In-Reply-To: <dc29b1131b84bef879cf68d1d1edae07f51ca3d2.camel@cybertec.at>
References: <17098-b960f3616c861f83@postgresql.org>
	<2468663.1626017338@sss.pgh.pa.us>
	<2984422.1626110456@sss.pgh.pa.us>
	<dc29b1131b84bef879cf68d1d1edae07f51ca3d2.camel@cybertec.at>

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Mon, 2021-07-12 at 13:20 -0400, Tom Lane wrote:
>> So my feeling about this is that switching snprintf.c's behavior
>> would produce some net gain in robustness for v12 and up, while
>> not making things any worse for the older branches.  I still hold
>> to the opinion that we've already flushed out all the cases of
>> passing NULL that we're likely to find via ordinary testing.

> New cases could be introduced in the future and might remain undetected.
> What about adding an Assert that gags on NULLs, but still printing them
> as "(null)"?  That would help find such problems in a debug build.

I think you're missing my main point, which is that it seems certain that
there are corner cases that do this *now*.  I'm proposing that we redefine
this as not being a crash case, full stop.

Now, what we don't have control of is what will happen in pre-v12
branches on platforms where we use the system's *printf.  However,
note what I wrote in the log for 0c62356cc:

    Per commit e748e902d, we appear to have little or no coverage in the
    buildfarm of machines that will dump core when asked to printf a
    null string pointer.

Thus it appears that a large fraction of the world is already either
using glibc or following glibc's lead on this point.  If we do likewise,
it will remove some crash cases and not introduce any new ones.

In hindsight I feel like 0c62356cc was an overreaction to the unusual
property e748e902d's bug had, namely that "(null)" was getting printed
in a place where it would not show up in any visible output.  Since
we certainly wouldn't consider that behavior OK if we saw it, you'd
really have to assume that there are more lurking bugs with that same
property in order to conclude that the Assert is worth its keep.

			regards, tom lane





view thread (13+ messages)  latest in thread

Message-ID: <3227956.1626186568@sss.pgh.pa.us>
Permalink:  ../3227956.1626186568@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/3227956.1626186568@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tgl@sss.pgh.pa.us, laurenz.albe@cybertec.at, pgsql-hackers@lists.postgresql.org
  Subject: Re: printf %s with NULL pointer (was Re: BUG #17098: Assert failed on composing an error message when adding a type to an extension being dropped)
  In-Reply-To: <3227956.1626186568@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox