Thread
Commits
-
Silence -Wmaybe-uninitialized compiler warnings in dbcommands.c.
- 3f6b3be39ca9 13.0 landed
-
"may be unused" warnings for gcc
Andres Freund <andres@anarazel.de> — 2017-02-20T14:41:18Z
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 -
Re: "may be unused" warnings for gcc
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-02-21T22:20:44Z
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
-
Re: "may be unused" warnings for gcc
Andres Freund <andres@anarazel.de> — 2017-02-22T03:17:22Z
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
-
Re: "may be unused" warnings for gcc
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-02-22T14:26:10Z
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
-
Re: [HACKERS] "may be unused" warnings for gcc
Andres Freund <andres@anarazel.de> — 2019-09-27T21:23:48Z
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