fixing more format truncation issues

Started by Peter Eisentrautalmost 8 years ago6 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

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

Attachments:

0001-Fix-more-format-truncation-issues.patchtext/plain; charset=UTF-8; name=0001-Fix-more-format-truncation-issues.patch; x-mac-creator=0; x-mac-type=0Download
From 99316e6db5a98f5daaaf61a75a01b88e7b2f39bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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

#2Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#1)
1 attachment(s)
Re: fixing more format truncation issues

On Wed, Feb 28, 2018 at 11:14:23PM -0500, Peter Eisentraut wrote:

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.

Good idea.

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.

OK, no complains about that.

@@ -89,7 +89,7 @@ static Datum
 build_pgstattuple_type(pgstattuple_type *stat, FunctionCallInfo fcinfo)
 {
 #define NCOLUMNS	9
-#define NCHARS		32
+#define NCHARS		314

So this one is caused by the output of %.2f...

Enabling them by default would generate some useless noise if the patch
is let as-is as a couple of them are not addressed. Please see the full
report attached. Is that intentional? I am using GCC 7.3 here.

interval.c: In function ‘AppendSeconds’:
interval.c:759:22: warning: ‘%0*d’ directive output between 1 and
2147483648 bytes may exceed minimum required size of 4095
[-Wformat-overflow=]
sprintf(cp, "%02d.%0*d", abs(sec), precision, (int) Abs(fsec));

pg_rusage.c:64:5: note: in expansion of macro ‘_’
_("CPU: user: %d.%02d s, system: %d.%02d s, elapsed: %d.%02d s"),
^
pg_rusage.c:63:2: note: ‘snprintf’ output between 51 and 108
bytes into a destination of size 100
snprintf(result, sizeof(result),
--
Michael

Attachments:

compile.txttext/plain; charset=utf-8Download
#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#2)
Re: fixing more format truncation issues

On 3/14/18 02:52, Michael Paquier wrote:

Enabling them by default would generate some useless noise if the patch
is let as-is as a couple of them are not addressed. Please see the full
report attached. Is that intentional? I am using GCC 7.3 here.

Well that's weird. Apparently, the warnings depend on a bit on build
options and other platform circumstances. That seems a bit cumbersome.
So I just committed the code changes but didn't actually add the
compiler options to configure.in. I'll close this patch now; it's not
worth going further right now.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#3)
Re: fixing more format truncation issues

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 3/14/18 02:52, Michael Paquier wrote:

Enabling them by default would generate some useless noise if the patch
is let as-is as a couple of them are not addressed. Please see the full
report attached. Is that intentional? I am using GCC 7.3 here.

Well that's weird. Apparently, the warnings depend on a bit on build
options and other platform circumstances. That seems a bit cumbersome.
So I just committed the code changes but didn't actually add the
compiler options to configure.in. I'll close this patch now; it's not
worth going further right now.

FWIW, I noticed while fooling with pre-release Fedora 28 that gcc 8.0.1
emits a whole boatload of these warnings by default. This patch doesn't
seem to have moved the needle very much, either. In a quick look, it
seemed like a lot of them were things we simply wouldn't be interested
in fixing ("yeah, the path length limit is MAXPGPATH, tough"). So
I'm thinking we're going to be needing -Wno-format-truncation soon.

regards, tom lane

#5Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#4)
Re: fixing more format truncation issues

On Thu, Mar 15, 2018 at 12:12:18PM -0400, Tom Lane wrote:

FWIW, I noticed while fooling with pre-release Fedora 28 that gcc 8.0.1
emits a whole boatload of these warnings by default. This patch doesn't
seem to have moved the needle very much, either. In a quick look, it
seemed like a lot of them were things we simply wouldn't be interested
in fixing ("yeah, the path length limit is MAXPGPATH, tough"). So
I'm thinking we're going to be needing -Wno-format-truncation soon.

Maybe it is worth a try on Debian as well. The base packages use GCC 7
but I can see that 8 is also available.
--
Michael

#6Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#4)
Re: fixing more format truncation issues

On 3/15/18 12:12, Tom Lane wrote:

FWIW, I noticed while fooling with pre-release Fedora 28 that gcc 8.0.1
emits a whole boatload of these warnings by default.

I have some patches for that. I will send them in once gcc 8 is a bit
closer to release.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services