In 6275f5d28a1577563f53f2171689d4f890a46881, we fixed warnings from the
options -Wformat-overflow and -Wformat-truncation, which are part of
-Wall in gcc 7.
Here, I propose to dial this up a bit by adding -Wformat-overflow=2
-Wformat-truncation=2, which use some more worst-case approaches for
estimating the buffer sizes.
AFAICT, the issues addressed here either can't really happen without
trying very hard, or would cause harmless output truncation. Still, it
seems good to clean this up properly and not rely on made-up buffer size
guesses that turn out to be wrong, even if we don't want to adopt the
warning options by default.
One issue that is of external interest is that I increase BGW_MAXLEN
from 64 to 96. Apparently, the old value would cause the bgw_name of
logical replication workers to be truncated in some circumstances. I
have also seen truncated background worker names with third-party
packages, so giving some more room here would be useful.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 99316e6db5a98f5daaaf61a75a01b88e7b2f39bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 28 Feb 2018 23:11:24 -0500
Subject: [PATCH] Fix more format truncation issues
Add compiler warning options -Wformat-overflow=2 -Wformat-truncation=2,
supported since GCC 7, and fix the warnings that they create.
The issues are all harmless, but some dubious coding patterns are
cleaned up.
---
configure | 71 ++++++++++++++++++++++++++++++++
configure.in | 3 ++
contrib/pgstattuple/pgstattuple.c | 2 +-
src/backend/commands/explain.c | 5 ++-
src/backend/utils/adt/dbsize.c | 2 +-
src/backend/utils/adt/float.c | 24 +++++------
src/backend/utils/adt/formatting.c | 33 +++++----------
src/backend/utils/misc/guc.c | 4 +-
src/bin/initdb/initdb.c | 6 +--
src/bin/pg_dump/pg_backup_archiver.c | 2 +-
src/bin/pg_dump/pg_backup_tar.c | 2 +-
src/bin/pgbench/pgbench.c | 4 +-
src/include/postmaster/bgworker.h | 2 +-
src/interfaces/libpq/fe-secure-openssl.c | 2 +-
src/pl/tcl/pltcl.c | 2 +-
15 files changed, 111 insertions(+), 53 deletions(-)
diff --git a/configure b/configure
index 7dcca506f8..48a7546513 100755
--- a/configure
+++ b/configure
@@ -4595,6 +4595,77 @@ if test x"$pgac_cv_prog_cc_cflags__Wformat_security" =
x"yes"; then
CFLAGS="$CFLAGS -Wformat-security"
fi
+ # gcc 7+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports
-Wformat-overflow=2" >&5
+$as_echo_n "checking whether $CC supports -Wformat-overflow=2... " >&6; }
+if ${pgac_cv_prog_cc_cflags__Wformat_overflow_2+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+CFLAGS="$pgac_save_CFLAGS -Wformat-overflow=2"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_cc_cflags__Wformat_overflow_2=yes
+else
+ pgac_cv_prog_cc_cflags__Wformat_overflow_2=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$pgac_cv_prog_cc_cflags__Wformat_overflow_2" >&5
+$as_echo "$pgac_cv_prog_cc_cflags__Wformat_overflow_2" >&6; }
+if test x"$pgac_cv_prog_cc_cflags__Wformat_overflow_2" = x"yes"; then
+ CFLAGS="$CFLAGS -Wformat-overflow=2"
+fi
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports
-Wformat-truncation=2" >&5
+$as_echo_n "checking whether $CC supports -Wformat-truncation=2... " >&6; }
+if ${pgac_cv_prog_cc_cflags__Wformat_truncation_2+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+CFLAGS="$pgac_save_CFLAGS -Wformat-truncation=2"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_cc_cflags__Wformat_truncation_2=yes
+else
+ pgac_cv_prog_cc_cflags__Wformat_truncation_2=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$pgac_cv_prog_cc_cflags__Wformat_truncation_2" >&5
+$as_echo "$pgac_cv_prog_cc_cflags__Wformat_truncation_2" >&6; }
+if test x"$pgac_cv_prog_cc_cflags__Wformat_truncation_2" = x"yes"; then
+ CFLAGS="$CFLAGS -Wformat-truncation=2"
+fi
+
# Disable strict-aliasing rules; needed for gcc 3.3+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports
-fno-strict-aliasing" >&5
$as_echo_n "checking whether $CC supports -fno-strict-aliasing... " >&6; }
diff --git a/configure.in b/configure.in
index 4d26034579..d46194235f 100644
--- a/configure.in
+++ b/configure.in
@@ -425,6 +425,9 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
+ # gcc 7+
+ PGAC_PROG_CC_CFLAGS_OPT([-Wformat-overflow=2])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wformat-truncation=2])
# Disable strict-aliasing rules; needed for gcc 3.3+
PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing])
# Disable optimizations that assume no overflow; needed for gcc 4.3+
diff --git a/contrib/pgstattuple/pgstattuple.c
b/contrib/pgstattuple/pgstattuple.c
index 7ca1bb24d2..b599b6ca21 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -89,7 +89,7 @@ static Datum
build_pgstattuple_type(pgstattuple_type *stat, FunctionCallInfo fcinfo)
{
#define NCOLUMNS 9
-#define NCHARS 32
+#define NCHARS 314
HeapTuple tuple;
char *values[NCOLUMNS];
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 900fa74e85..f0dfef5a86 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3337,10 +3337,11 @@ void
ExplainPropertyFloat(const char *qlabel, double value, int ndigits,
ExplainState *es)
{
- char buf[256];
+ char *buf;
- snprintf(buf, sizeof(buf), "%.*f", ndigits, value);
+ buf = psprintf("%.*f", ndigits, value);
ExplainProperty(qlabel, buf, true, es);
+ pfree(buf);
}
/*
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 834a10485f..07e5e78caa 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -86,7 +86,7 @@ calculate_database_size(Oid dbOid)
DIR *dirdesc;
struct dirent *direntry;
char dirpath[MAXPGPATH];
- char pathname[MAXPGPATH + 12 +
sizeof(TABLESPACE_VERSION_DIRECTORY)];
+ char pathname[MAXPGPATH + 21 +
sizeof(TABLESPACE_VERSION_DIRECTORY)];
AclResult aclresult;
/*
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index aadb92de66..6522c0816e 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -44,10 +44,6 @@ static const uint32 nan[2] = {0xffffffff, 0x7fffffff};
#define NAN (*(const double *) nan)
#endif
-/* not sure what the following should be, but better to make it
over-sufficient */
-#define MAXFLOATWIDTH 64
-#define MAXDOUBLEWIDTH 128
-
/*
* check to see if a float4/8 val has underflowed or overflowed
*/
@@ -360,18 +356,18 @@ Datum
float4out(PG_FUNCTION_ARGS)
{
float4 num = PG_GETARG_FLOAT4(0);
- char *ascii = (char *) palloc(MAXFLOATWIDTH + 1);
+ char *ascii;
if (isnan(num))
- PG_RETURN_CSTRING(strcpy(ascii, "NaN"));
+ PG_RETURN_CSTRING(pstrdup("NaN"));
switch (is_infinite(num))
{
case 1:
- strcpy(ascii, "Infinity");
+ ascii = pstrdup("Infinity");
break;
case -1:
- strcpy(ascii, "-Infinity");
+ ascii = pstrdup("-Infinity");
break;
default:
{
@@ -380,7 +376,7 @@ float4out(PG_FUNCTION_ARGS)
if (ndig < 1)
ndig = 1;
- snprintf(ascii, MAXFLOATWIDTH + 1, "%.*g",
ndig, num);
+ ascii = psprintf("%.*g", ndig, num);
}
}
@@ -596,18 +592,18 @@ float8out(PG_FUNCTION_ARGS)
char *
float8out_internal(double num)
{
- char *ascii = (char *) palloc(MAXDOUBLEWIDTH + 1);
+ char *ascii;
if (isnan(num))
- return strcpy(ascii, "NaN");
+ return pstrdup("NaN");
switch (is_infinite(num))
{
case 1:
- strcpy(ascii, "Infinity");
+ ascii = pstrdup("Infinity");
break;
case -1:
- strcpy(ascii, "-Infinity");
+ ascii = pstrdup("-Infinity");
break;
default:
{
@@ -616,7 +612,7 @@ float8out_internal(double num)
if (ndig < 1)
ndig = 1;
- snprintf(ascii, MAXDOUBLEWIDTH + 1, "%.*g",
ndig, num);
+ ascii = psprintf("%.*g", ndig, num);
}
}
diff --git a/src/backend/utils/adt/formatting.c
b/src/backend/utils/adt/formatting.c
index b8bd4caa3e..1a1088711c 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -117,13 +117,6 @@
#define DCH_MAX_ITEM_SIZ 12 /* max localized day name
*/
#define NUM_MAX_ITEM_SIZ 8 /* roman number (RN has 15
chars) */
-/* ----------
- * More is in float.c
- * ----------
- */
-#define MAXFLOATWIDTH 60
-#define MAXDOUBLEWIDTH 500
-
/* ----------
* Format parser structs
@@ -3911,9 +3904,7 @@ do_to_timestamp(text *date_txt, text *fmt,
tmfc.tzm < 0 || tmfc.tzm >= MINS_PER_HOUR)
DateTimeParseError(DTERR_TZDISP_OVERFLOW, date_str,
"timestamp");
- tz = palloc(7);
-
- snprintf(tz, 7, "%c%02d:%02d",
+ tz = psprintf("%c%02d:%02d",
tmfc.tzsign > 0 ? '+' : '-', tmfc.tzh,
tmfc.tzm);
tm->tm_zone = tz;
@@ -4135,7 +4126,7 @@ int_to_roman(int number)
num = 0;
char *p = NULL,
*result,
- numstr[5];
+ numstr[12];
result = (char *) palloc(16);
*result = '\0';
@@ -5441,8 +5432,7 @@ int4_to_char(PG_FUNCTION_ARGS)
/* we can do it easily because float8 won't lose any precision
*/
float8 val = (float8) value;
- orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
- snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, val);
+ orgnum = (char *) psprintf("%+.*e", Num.post, val);
/*
* Swap a leading positive sign for a space.
@@ -5641,7 +5631,6 @@ float4_to_char(PG_FUNCTION_ARGS)
numstr = orgnum = int_to_roman((int) rint(value));
else if (IS_EEEE(&Num))
{
- numstr = orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
if (isnan(value) || is_infinite(value))
{
/*
@@ -5655,7 +5644,7 @@ float4_to_char(PG_FUNCTION_ARGS)
}
else
{
- snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post,
value);
+ numstr = orgnum = psprintf("%+.*e", Num.post, value);
/*
* Swap a leading positive sign for a space.
@@ -5679,8 +5668,7 @@ float4_to_char(PG_FUNCTION_ARGS)
Num.pre += Num.multi;
}
- orgnum = (char *) palloc(MAXFLOATWIDTH + 1);
- snprintf(orgnum, MAXFLOATWIDTH + 1, "%.0f", fabs(val));
+ orgnum = (char *) psprintf("%.0f", fabs(val));
numstr_pre_len = strlen(orgnum);
/* adjust post digits to fit max float digits */
@@ -5688,7 +5676,7 @@ float4_to_char(PG_FUNCTION_ARGS)
Num.post = 0;
else if (numstr_pre_len + Num.post > FLT_DIG)
Num.post = FLT_DIG - numstr_pre_len;
- snprintf(orgnum, MAXFLOATWIDTH + 1, "%.*f", Num.post, val);
+ orgnum = psprintf("%.*f", Num.post, val);
if (*orgnum == '-')
{ /* < 0 */
@@ -5747,7 +5735,6 @@ float8_to_char(PG_FUNCTION_ARGS)
numstr = orgnum = int_to_roman((int) rint(value));
else if (IS_EEEE(&Num))
{
- numstr = orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
if (isnan(value) || is_infinite(value))
{
/*
@@ -5761,7 +5748,7 @@ float8_to_char(PG_FUNCTION_ARGS)
}
else
{
- snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post,
value);
+ numstr = orgnum = (char *) psprintf("%+.*e", Num.post,
value);
/*
* Swap a leading positive sign for a space.
@@ -5784,15 +5771,15 @@ float8_to_char(PG_FUNCTION_ARGS)
val = value * multi;
Num.pre += Num.multi;
}
- orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
- numstr_pre_len = snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%.0f",
fabs(val));
+ orgnum = psprintf("%.0f", fabs(val));
+ numstr_pre_len = strlen(orgnum);
/* adjust post digits to fit max double digits */
if (numstr_pre_len >= DBL_DIG)
Num.post = 0;
else if (numstr_pre_len + Num.post > DBL_DIG)
Num.post = DBL_DIG - numstr_pre_len;
- snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%.*f", Num.post, val);
+ orgnum = psprintf("%.*f", Num.post, val);
if (*orgnum == '-')
{ /* < 0 */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1db7845d5a..b1932e5fd6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10527,7 +10527,7 @@ check_cluster_name(char **newval, void **extra,
GucSource source)
static const char *
show_unix_socket_permissions(void)
{
- static char buf[8];
+ static char buf[12];
snprintf(buf, sizeof(buf), "%04o", Unix_socket_permissions);
return buf;
@@ -10536,7 +10536,7 @@ show_unix_socket_permissions(void)
static const char *
show_log_file_mode(void)
{
- static char buf[8];
+ static char buf[12];
snprintf(buf, sizeof(buf), "%04o", Log_file_mode);
return buf;
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2efd3b75b1..c0ea7df3aa 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1009,12 +1009,12 @@ static char *
pretty_wal_size(int segment_count)
{
int sz = wal_segment_size_mb * segment_count;
- char *result = pg_malloc(11);
+ char *result = pg_malloc(14);
if ((sz % 1024) == 0)
- snprintf(result, 11, "%dGB", sz / 1024);
+ snprintf(result, 14, "%dGB", sz / 1024);
else
- snprintf(result, 11, "%dMB", sz);
+ snprintf(result, 14, "%dMB", sz);
return result;
}
diff --git a/src/bin/pg_dump/pg_backup_archiver.c
b/src/bin/pg_dump/pg_backup_archiver.c
index fc233a608f..83c976eaf7 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -1532,7 +1532,7 @@ SetOutput(ArchiveHandle *AH, const char *filename, int
compression)
#ifdef HAVE_LIBZ
if (compression != 0)
{
- char fmode[10];
+ char fmode[14];
/* Don't use PG_BINARY_x since this is zlib */
sprintf(fmode, "wb%d", compression);
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index ef9f7145b1..007be1298f 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -335,7 +335,7 @@ tarOpen(ArchiveHandle *AH, const char *filename, char mode)
TAR_MEMBER *tm;
#ifdef HAVE_LIBZ
- char fmode[10];
+ char fmode[14];
#endif
if (mode == 'r')
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d4209421f5..18ced8a2b2 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3601,7 +3601,7 @@ parseQuery(Command *cmd)
p = sql;
while ((p = strchr(p, ':')) != NULL)
{
- char var[12];
+ char var[13];
char *name;
int eaten;
@@ -5443,7 +5443,7 @@ threadRun(void *arg)
sqlat,
lag,
stdev;
- char tbuf[64];
+ char tbuf[315];
/*
* Add up the statistics of all threads.
diff --git a/src/include/postmaster/bgworker.h
b/src/include/postmaster/bgworker.h
index 0c04529f47..a8753df8d1 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -82,7 +82,7 @@ typedef enum
#define BGW_DEFAULT_RESTART_INTERVAL 60
#define BGW_NEVER_RESTART -1
-#define BGW_MAXLEN 64
+#define BGW_MAXLEN 96
#define BGW_EXTRALEN 128
typedef struct BackgroundWorker
diff --git a/src/interfaces/libpq/fe-secure-openssl.c
b/src/interfaces/libpq/fe-secure-openssl.c
index cade4e157c..127122563c 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1436,7 +1436,7 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
if (strcmp(attribute_name, "key_bits") == 0)
{
- static char sslbits_str[10];
+ static char sslbits_str[12];
int sslbits;
SSL_get_cipher_bits(conn->ssl, &sslbits);
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 5df4dfdf55..a3d9a8759f 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1459,7 +1459,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
Datum prosrcdatum;
bool isnull;
char *proc_source;
- char buf[32];
+ char buf[48];
Tcl_Interp *interp;
int i;
int tcl_rc;
--
2.16.2
