[HACKERS] "may be unused" warnings for gcc - Mailing list pgsql-hackers

From Andres Freund
Subject [HACKERS] "may be unused" warnings for gcc
Date
Msg-id 20170220144118.qqq3zp2lnuq52e4c@alap3.anarazel.de
Whole thread Raw
Responses Re: [HACKERS] "may be unused" warnings for gcc
List 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
thisfunction [-Wmaybe-uninitialized]   errfinish rest; \   ^~~~~~~~~
 
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: ‘context.location2’ was declared here
assign_collations_contextcontext;                           ^~~~~~~
 
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
thisfunction [-Wmaybe-uninitialized]   errfinish rest; \   ^~~~~~~~~
 
/home/andres/src/postgresql/src/backend/parser/parse_collate.c:210:28: note: ‘context.collation2’ was declared here
assign_collations_contextcontext;                           ^~~~~~~
 

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
thisfunction [-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
uninitializedin 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
TextPositionStatestate;                   ^~~~~
 
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1344:9: warning: ‘state.wstr2’ may be used uninitialized in
thisfunction [-Wmaybe-uninitialized]     if (nptr == needle)        ^
 
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: ‘state.wstr2’ was declared here
TextPositionStatestate;                   ^~~~~
 
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: warning: ‘state.wstr1’ may be used uninitialized
inthis function [-Wmaybe-uninitialized]
 
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1288:9: warning: ‘state.str2’ may be used uninitialized in
thisfunction [-Wmaybe-uninitialized]     if (nptr == needle)        ^
 
/home/andres/src/postgresql/src/backend/utils/adt/varlena.c:1099:20: note: ‘state.str2’ was declared here
TextPositionStatestate;                   ^~~~~
 

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
thisfunction [-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



pgsql-hackers by date:

Previous
From: Aleksander Alekseev
Date:
Subject: Re: [HACKERS] [PATCH] Suppress Clang 3.9 warnings
Next
From: Kuntal Ghosh
Date:
Subject: Re: [HACKERS] Passing query string to workers