pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed"may be unused" warnings for gcc
5+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2019-09-27 3f6b3be39c Silence -Wmaybe-uninitialized compiler warnings in dbcommands.c.
* "may be unused" warnings for gcc
@ 2017-02-20 14:41 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 5+ messages in thread
From: Andres Freund @ 2017-02-20 14:41 UTC (permalink / raw)
To: pgsql-hackers
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?
/home/andres/src/postgresql/src/backend/storage/ipc/shm_mq.c: In function ‘shm_mq_receive’:
/home/andres/src/postgresql/src/backend/storage/ipc/shm_mq.c:705:3: warning: ‘rawdata’ may be used uninitialized in this function [-Wmaybe-uninitialized]
memcpy(&mqh->mqh_buffer[mqh->mqh_partial_bytes], rawdata, rb);
That one I'm not surprised about at all - pretty hard to figure out that
rawdata has to be set at that point.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: "may be unused" warnings for gcc
@ 2017-02-21 22:20 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 5+ messages in thread
From: Peter Eisentraut @ 2017-02-21 22:20 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; pgsql-hackers
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: "may be unused" warnings for gcc
@ 2017-02-22 03:17 Andres Freund <andres@anarazel.de>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 5+ messages in thread
From: Andres Freund @ 2017-02-22 03:17 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: "may be unused" warnings for gcc
@ 2017-02-22 14:26 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 5+ messages in thread
From: Peter Eisentraut @ 2017-02-22 14:26 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
0001-Silence-compiler-warnings-from-gcc-O3.patch (text/x-patch, 2.8K) download
From 2a73d3ac095435ecbe3aefff7bcecc292675e167 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
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
* Re: [HACKERS] "may be unused" warnings for gcc
@ 2019-09-27 21:23 Andres Freund <andres@anarazel.de>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 0 replies; 5+ messages in thread
From: Andres Freund @ 2019-09-27 21:23 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
Hi,
On 2017-02-22 09:26:10 -0500, Peter Eisentraut wrote:
> 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.
I pushed the subset that I still see locally with gcc -O3.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2019-09-27 21:23 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 14:41 "may be unused" warnings for gcc Andres Freund <andres@anarazel.de>
2017-02-21 22:20 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-02-22 03:17 ` Andres Freund <andres@anarazel.de>
2017-02-22 14:26 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-09-27 21:23 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox