Re: GCC 8 warnings

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Devrim Gündüz <devrim(at)gunduz(dot)org>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: GCC 8 warnings
Date: 2018-06-16 17:29:55
Message-ID: 21789.1529170195@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2018-04-28 12:16:39 -0400, Tom Lane wrote:
>> In the meantime, I think our response to GCC 8 should just be to
>> enable -Wno-format-truncation. Certainly so in the back branches.
>> There isn't one of these changes that is actually fixing a bug,
>> which to me says that that warning is unhelpful.

> Agreed. Or at least turn down its aggressiveness to the cases where it's
> actually sure truncation happens.

I got around to poking into this today. Unfortunately, it doesn't seem
that there's any more-conservative level of -Wformat-truncation available.
Likewise for -Wstringop-truncation, which is the other rich new source
of useless warnings (it appears to be predicated on the assumption that
you never actually want the defined semantics of strncpy). Hence,
I propose the attached patch to disable these warnings if the compiler
knows the switch for them. I did not turn them off for CXX though;
anyone think there's a need to?

Testing with Fedora 28's gcc (currently 8.1.1), this leaves three new
warnings, which seem worth fixing. Two of them are gratuitous use of
strncpy where memcpy would serve, in ecpg, and one is an sprintf in
pg_waldump that should be snprintf for safety's sake:

common.c: In function 'pgtypes_fmt_replace':
common.c:48:5: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-overflow=]
strncpy(*output, replace_val.str_val, i + 1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
common.c:42:8: note: length computed here
i = strlen(replace_val.str_val);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'get_char_item',
inlined from 'ECPGget_desc' at descriptor.c:334:10:
descriptor.c:221:6: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-overflow=]
strncpy(variable->arr, value, strlen(value));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compat.c: In function 'timestamptz_to_str':
compat.c:61:19: warning: '%06d' directive writing between 6 and 7 bytes into a region of size between 0 and 128 [-Wformat-overflow=]
sprintf(buf, "%s.%06d %s", ts, (int) (dt % USECS_PER_SEC), zone);
^~~~
compat.c:61:15: note: directive argument in the range [-999999, 999999]
sprintf(buf, "%s.%06d %s", ts, (int) (dt % USECS_PER_SEC), zone);
^~~~~~~~~~~~
compat.c:61:2: note: 'sprintf' output between 9 and 266 bytes into a destination of size 129
sprintf(buf, "%s.%06d %s", ts, (int) (dt % USECS_PER_SEC), zone);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Probably all of this ought to be back-patched as applicable, since
people will doubtless try to compile back branches with gcc 8.

regards, tom lane

Attachment Content-Type Size
no-truncation-warnings.patch text/x-diff 816 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2018-06-16 18:52:18 Re: GCC 8 warnings
Previous Message Tomas Vondra 2018-06-16 15:59:24 Re: POC: GROUP BY optimization