Re: [HACKERS] "may be unused" warnings for gcc

[Peter Eisentraut]

On 2/21/17 22:17, Andres Freund wrote:
> I've not run comparisons this year, but late last year I was seeing > 5%
> < 10% benefits - that seems plenty enough to care.

You mean the 5-minute benchmarks on my laptop are not representative? ;-)

Here is a patch that I had lying around that clears the compiler
warnings under -O3 for me.  It seems that they are a subset of what you
are seeing.  Plausibly, as compilers are doing more analysis in larger
scopes, we can expect to see more of these.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 2a73d3ac095435ecbe3aefff7bcecc292675e167 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 22 Feb 2017 09:23:40 -0500
Subject: [PATCH] Silence compiler warnings from gcc -O3

---
 src/backend/commands/dbcommands.c | 13 +++--
 src/backend/utils/adt/varlena.c   |  6 ++
 src/backend/utils/misc/guc.c  |  2 +-
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 1ebacbc24f..2a23f634c5 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -27,6 +27,7 @@
 #include "access/genam.h"
 #include "access/heapam.h"
 #include "access/htup_details.h"
+#include "access/multixact.h"
 #include "access/xact.h"
 #include "access/xloginsert.h"
 #include "access/xlogutils.h"
@@ -103,14 +104,14 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 	Relation	rel;
 	Oid			src_dboid;
 	Oid			src_owner;
-	int			src_encoding;
-	char	   *src_collate;
-	char	   *src_ctype;
+	int			src_encoding = -1;
+	char	   *src_collate = NULL;
+	char	   *src_ctype = NULL;
 	bool		src_istemplate;
 	bool		src_allowconn;
-	Oid			src_lastsysoid;
-	TransactionId src_frozenxid;
-	MultiXactId src_minmxid;
+	Oid			src_lastsysoid = InvalidOid;
+	TransactionId src_frozenxid = InvalidTransactionId;
+	MultiXactId src_minmxid = InvalidMultiXactId;
 	Oid			src_deftablespace;
 	volatile Oid dst_deftablespace;
 	Relation	pg_database_rel;
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 28b5745ba8..7da7535b3b 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1129,6 +1129,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state)
 		state->use_wchar = false;
 		state->str1 = VARDATA_ANY(t1);
 		state->str2 = VARDATA_ANY(t2);
+		state->wstr1 = 0;
+		state->wstr2 = 0;
 		state->len1 = len1;
 		state->len2 = len2;
 	}
@@ -1144,6 +1146,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state)
 		len2 = pg_mb2wchar_with_len(VARDATA_ANY(t2), p2, len2);
 
 		state->use_wchar = true;
+		state->str1 = 0;
+		state->str2 = 0;
 		state->wstr1 = p1;
 		state->wstr2 = p2;
 		state->len1 = len1;
@@ -1227,6 +1231,8 @@ text_position_setup(text *t1, text *t2, TextPositionState *state)
 state->skiptable[wstr2[i] & skiptablemask] = last - i;
 		}
 	}
+	else
+		state->skiptablemask = 255;
 }
 
 static int
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 24771389c8..107e1f2957 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -9275,7 +9275,7 @@ RestoreGUCState(void *gucstate)
 	char	   *varname,
 			   *varvalue,
 			   *varsourcefile;
-	int			varsourceline;
+	int			varsourceline = -1;
 	GucSource	varsource;
 	GucContext	varscontext;
 	char	   *srcptr = (char *) gucstate;
-- 
2.11.1


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] "may be unused" warnings for gcc

[Andres Freund]

On 2017-02-21 17:20:44 -0500, Peter Eisentraut wrote:
> On 2/20/17 09:41, Andres Freund wrote:
> > When building with a new-ish gcc (6.3.0 right now, but I've seen this
> > for a while) with optimization I get a number of warnings:
> 
> These all look like related to inlining/-O3.
> 
> I have attempted to fix these in the past, but I have found that -O3
> doesn't get any performance improvement, so I haven't bothered lately.

I've not run comparisons this year, but late last year I was seeing > 5%
< 10% benefits - that seems plenty enough to care.

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] "may be unused" warnings for gcc

[Peter Eisentraut]

On 2/20/17 09:41, Andres Freund wrote:
> When building with a new-ish gcc (6.3.0 right now, but I've seen this
> for a while) with optimization I get a number of warnings:

These all look like related to inlining/-O3.

I have attempted to fix these in the past, but I have found that -O3
doesn't get any performance improvement, so I haven't bothered lately.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] "may be unused" warnings for gcc

[Andres Freund]

Hi,

When building with a new-ish gcc (6.3.0 right now, but I've seen this
for a while) with optimization I get a number of warnings:

In file included from /home/andres/src/postgresql/src/include/postgres.h:48:0,
 from 
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:41:
/home/andres/src/postgresql/src/backend/parser/parse_collate.c: In function 
‘select_common_collation’:
/home/andres/src/postgresql/src/include/utils/elog.h:107:4: warning: 
‘context.location2’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
errfinish rest; \
^
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: 
‘context.location2’ was declared here
  assign_collations_context context;
^~~
In file included from /home/andres/src/postgresql/src/include/postgres.h:48:0,
 from 
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:41:
/home/andres/src/postgresql/src/include/utils/elog.h:107:4: warning: 
‘context.collation2’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
errfinish rest; \
^
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: 
‘context.collation2’ was declared here
  assign_collations_context context;
^~~

While I believe these are false positives, I am not surprised that the
compiler can't see that.  select_common_collation() initializes some
fields of assign_collations_context, but not others.  There's several
branches out of assign_collations_walker that return without setting
ocllation2/location2. I think that's currently harmless because
it looks like select_common_collation() won't enter the context.strength
== COLLATE_CONFLICT branch in that case - but it's certainly hard to
see.


In file included from 
/home/andres/src/postgresql/src/backend/commands/dbcommands.c:20:0:
/home/andres/src/postgresql/src/backend/commands/dbcommands.c: In function 
‘createdb’:
/home/andres/src/postgresql/src/include/postgres.h:529:35: warning: 
‘src_minmxid’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 #define TransactionIdGetDatum(X) ((Datum) SET_4_BYTES((X)))
   ^
/home/andres/src/postgresql/src/backend/commands/dbcommands.c:113:14: note: 
‘src_minmxid’ was declared here
  MultiXactId src_minmxid;
  ^~~
(and the same for src_frozenxid, src_lastsysoid, ...)

It appears that the loop in get_db_info() is too complex for gcc.
Replacing the !HeapTupleIsValid(tuple) break; with a heap_close() and
direct return fixes those.


/home/andres/src/postgresql/src/backend/utils/misc/guc.c: In function 
‘RestoreGUCState’:
/home/andres/src/postgresql/src/backend/utils/misc/guc.c:6619:21: warning: 
‘varsourceline’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
  record->sourceline = sourceline;
  ~~~^~~~
/home/andres/src/postgresql/src/backend/utils/misc/guc.c:9279:8: note: 
‘varsourceline’ was declared here
  int   varsourceline;
^

Not sure what the problem is here - even if the varsourcefile[0] test in
RestoreGUCState is stored in a local variable that's also checked before
the set_config_sourcefile() branch, it warns.  Initializing
varsourceline to 0 works and seems reasonable.


/home/andres/src/postgresql/src/backend/utils/adt/varlena.c: In function 
‘text_position’:
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1358:36: warning: 
‘state.skiptablemask’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
 hptr += state->skiptable[*hptr & skiptablemask];
  ~~^~~
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: 
‘state.skiptablemask’ was declared here
  TextPositionState state;
^
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1344:9: warning: 
‘state.wstr2’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  if (nptr == needle)
 ^
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: 
‘state.wstr2’ was declared here
  TextPositionState state;
^
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: warning: 
‘state.wstr1’ may be used uninitialized in this function [-Wmaybe-uninitialized]
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1288:9: warning: 
‘state.str2’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  if (nptr == needle)
 ^
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: 
‘state.str2’ was declared here
  TextPositionState state;
^

No idea what exactly triggers this, but zero-initializing
TextPositionState helps. What confuses me is that doing so in
text_position() is sufficient - the other uses don't trigger a warning?