Re: printf format selection vs. reality - Mailing list pgsql-hackers

From Tom Lane
Subject Re: printf format selection vs. reality
Date
Msg-id 8383.1527107335@sss.pgh.pa.us
Whole thread Raw
In response to Re: printf format selection vs. reality  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: printf format selection vs. reality  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: printf format selection vs. reality  (Thomas Munro <thomas.munro@enterprisedb.com>)
List pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-May-23, Tom Lane wrote:
>> The practical alternatives seem to be to avoid %z in frontend code,
>> or to invent a macro SIZE_T_MODIFIER and use it like INT64_MODIFIER.
>> Either one will be extremely error-prone, I'm afraid, unless we can
>> find a way to get compiler warnings for violations.

> Usage of %z outside safe-known seems really limited.  It would be sad to
> force SIZE_T_MODIFIER for elog calls (where it is prevalent) just for
> the benefit of usage outside of elog on fringe platforms -- you're right
> that we do have a few cases of %z under fprintf() already.  The good
> news is that AFAICS those strings are not translatable today, so
> changing only those to SIZE_T_MODIFIER (and leaving alone those using
> elog) is maybe not such a big deal.  I think those are dshash.c, dsa.c,
> slab.c and aset.c only.

Yeah, I just went through things myself, and concluded that right now
the only hazards are in debug code such as dsa_dump().  So I think that
(a) we don't have a problem we have to fix right now, and (b) going
over to SIZE_T_MODIFIER seems like more trouble than it'd be worth.
Still, this seems like something that will certainly bite us eventually
if we don't install some kind of check.

> (I assume without checking that with the stringinfo API it's OK to use %z).

It is, that goes to snprintf.

> Can't we raise warnings on such usages with an archetype change?  (Hm,
> is it possible to change archetype for fprintf?)

The problem is to get a compiler that thinks that %z is a violation
of *any* archetype.  gaur's compiler does think that, but it has no
archetype that does accept %z, so that's little help (I've had it
building with -Wno-format since we added %z).

It might be possible for me to install a fractionally-newer compiler
on gaur's host and get usable warnings that way.  I think however
that a more practical approach is likely to be to depend on the
Windows/gcc buildfarm members, where (if gcc is correctly installed
and doing what it's supposed to) we should find that %z is accepted
by the gnu_printf archetype but not the plain printf one.  So I wish
somebody would try out the patch in <2975.1526862605@sss.pgh.pa.us>
on MinGW.  It would also be good to find out whether MSVC can be
persuaded to check printf strings.

            regards, tom lane

PS: per the above, the patch in <2975.1526862605@sss.pgh.pa.us>
would need to be adjusted to use gnu_printf on the stringinfo
functions, if we don't want complaints about %z.  This opens
the question of whether we want to allow %m there ...


pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: printf format selection vs. reality
Next
From: Christoph Berg
Date:
Subject: Re: PG11 jit failing on ppc64el