Created attachment 40653 [details] test source Using g++ 7.0.1 20170202 (trunk), compiling this code: ============== #include <stdlib.h> struct ui_file {}; ui_file stream_v, stream_q; struct gdb_disassembler { gdb_disassembler (struct ui_file *file) : m_stream (file) {} struct ui_file *m_stream; }; /* Test disassembly of one instruction. */ struct gdb_disassembler_test : public gdb_disassembler { const bool verbose = false; gdb_disassembler_test () : gdb_disassembler (verbose ? &stream_v : &stream_q) // bug here {} void use_stream (); }; void func () { gdb_disassembler_test di; di.use_stream (); } ==== with: g++ -std=gnu++11 uninitialized.cc -c -o uninitialized -O2 does not produce any warning. However, the member initializer here at: gdb_disassembler_test () : gdb_disassembler (verbose ? &stream_v : &stream_q) // bug here {} is passing the yet-uninitialized "gdb_disassembler_test::verbose" to the gdb_disassembler ctor. This is a bug, because even though the "verbose" field has an in-class initializer, base classes are initialized before members of the current class are initialized. The frontend end obviously must know this, so it should be able to detect the bug and warn, regardless of optimization level. (At least, when passing by value. Passing by reference/pointer may have legitimate uses.) (TBC, g++ does not warn at any optimization level.) Looks like a regression. I tried: g++ 7.0.1 20170202 (trunk) g++ 5.3.1 (Fedora 23) g++ 4.8.5 g++ 4.7.4 clang++ 3.7 All but g++ 7 warned at -O2. None warned at -O0. clang++ manages to warn even at -O0. g++ 5.3.1 (-O2): $ g++ -std=gnu++11 -O2 -Wall -Wextra -Wuninitialized gcc-bug.cc -c -o gcc-bug gcc-bug.cc: In function ‘void func()’: gcc-bug.cc:19:25: warning: ‘di.gdb_disassembler_test::verbose’ is used uninitialized in this function [-Wuninitialized] : gdb_disassembler (verbose ? &stream_v : &stream_q) // bug here ^ gcc-bug.cc:28:25: note: ‘di’ was declared here gdb_disassembler_test di; ^ g++ 4.7 (-O2): $ /opt/gcc-4.7/bin/g++ -std=gnu++11 -O2 -Wall -Wextra -Wuninitialized gcc-bug.cc -c -o gcc-bug gcc-bug.cc: In function ‘void func()’: gcc-bug.cc:19:56: warning: ‘di.gdb_disassembler_test::verbose’ is used uninitialized in this function [-Wuninitialized] gcc-bug.cc:28:25: note: ‘di’ was declared here clang++ (-O0): $ clang++ -std=gnu++11 -O0 -Wall -Wextra -Wuninitialized gcc-bug.cc -c -o gcc-bug gcc-bug.cc:19:25: warning: field 'verbose' is uninitialized when used here [-Wuninitialized] : gdb_disassembler (verbose ? &stream_v : &stream_q) // bug here ^ 1 warning generated.
Note, if we add a use of gdb_disassembler::m_stream somewhere, like: gdb_disassembler_test () : gdb_disassembler (verbose == 123 ? &stream_v : &stream_q) // bug here { if (m_stream != &stream_v) abort (); } then g++ 7 warns at -O2: $ /opt/gcc/bin/g++ -std=gnu++11 -O2 -Wall -Wextra -Wuninitialized gcc-bug.cc -o gcc-bug gcc-bug.cc: In function ‘int main()’: gcc-bug.cc:19:63: warning: ‘di.gdb_disassembler_test::verbose’ is used uninitialized in this function [-Wuninitialized] : gdb_disassembler (verbose == 123 ? &stream_v : &stream_q) // bug here ^ gcc-bug.cc:29:25: note: ‘di.gdb_disassembler_test::verbose’ was declared here gdb_disassembler_test di; ^~ Unfortunately, the compilation unit that has this but in gdb doesn't have any use like that, so the bug went unnoticed. To reiterate, I think the frontend _should_ be able to warn even without seeing the use of m_stream. I.e., I don't think this bug is the same category as the other truckload of bugs about missing -Winitialized opportunities at -O0 related to not running some parts of the optimization pipeline.
Confirmed. Bisection points to r222135 (gcc 6.0.0): ------------------------------------------------------------------------ r222135 | jason | 2015-04-15 17:17:29 -0400 (Wed, 15 Apr 2015) | 5 lines * constexpr.c (cxx_eval_store_expression): Ignore clobbers. (build_constexpr_constructor_member_initializers): Loop to find the BIND_EXPR. * decl.c (start_preparsed_function): Clobber the object at the beginning of a constructor. A somewhat simplified test case: struct A { A (int); }; struct B: A { const bool x = true; B (): A (x ? 3 : 7) { } }; void f (void*); void g () { B b; f (&b); }
Compiling with -fno-lifetime-dse brings back the warning, so it seems that the -Wuninitialized code is improperly treating the clobber as initialization.
Changing component since this seems to be a tree-ssa-uninit.c issue.
I'll have a look.
Created attachment 40784 [details] gcc7-pr79345.patch Untested fix. That said, I think (most likely just for GCC8) it would be very much beneficial to walk_aliased_defs as the comment says, and if we run into aliased clobber that must overlap the read ref, warn (and if we don't find any aliased store too), with some cap on number of alias oracle queries.
Created attachment 40796 [details] untested patch w/o limiting the oracle walk Extra warns for FAIL: gcc.dg/uninit-B-O0.c (test for excess errors) FAIL: gcc.dg/uninit-pr19430-2.c (test for excess errors) Excess errors: /space/rguenther/src/svn/gcc-7-branch/gcc/testsuite/gcc.dg/uninit-B-O0.c:13:5: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized] Excess errors: /space/rguenther/src/svn/gcc-7-branch/gcc/testsuite/gcc.dg/uninit-pr19430-2.c:13:7: warning: 'i' may be used uninitialized in this function [-Wmaybe-uninitialized] which is kind-of expected (TREE_NO_WARNING wouldn't work on non-shared trees in general). Maybe we should try to mark the base if it is a decl? But that might miss cases, say for { X a; if (flag) ... = a.x; else .... = a.y; } we'd warn for a.x but not a.y. To get an idea about verbosity I'll bootstrap the patch (w/o the limiting). Comments welcome.
11:14 < richi> In function ‘void branch_target_load_optimize(bool)’: 11:14 < richi> cc1plus: error: ‘call_saved’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 11:14 < richi> cc1plus: error: ‘*((void*)& call_saved +8)’ may be used uninitialized in this function [-Werror=maybe-uninitialized] In file included from /space/rguenther/src/svn/trunk/gcc/coretypes.h:365:0, from /space/rguenther/src/svn/trunk/gcc/fixed-value.c:22: /space/rguenther/src/svn/trunk/gcc/wide-int.h: In function ‘void fixed_from_string(fixed_value*, const char*, machine_mode)’: /space/rguenther/src/svn/trunk/gcc/wide-int.h:818:49: error: ‘*((void*)& w +34359738360)’ may be used uninitialized in this function [-Werror=maybe-uninitialized] unsigned HOST_WIDE_INT high = this->get_val ()[len - 1]; ~~~~~~~~~~~~~~~~^ /space/rguenther/src/svn/trunk/gcc/wide-int.h: In function ‘bool fixed_convert_from_real(fixed_value*, machine_mode, const real_value*, bool)’: /space/rguenther/src/svn/trunk/gcc/wide-int.h:818:49: error: ‘*((void*)& w +34359738360)’ may be used uninitialized in this function [-Werror=maybe-uninitialized] unsigned HOST_WIDE_INT high = this->get_val ()[len - 1]; ~~~~~~~~~~~~~~~~^ the above is from f->data.low = w.elt (0); and generic_wide_int <storage>::elt (unsigned int i) const { if (i >= this->get_len ()) return sign_mask (); else return this->get_val ()[i]; resulting in 0 >= get_len () (which is get_len == 0). And then we have 11:32 < jakub> richi: looks like a bug in genemit.c 11:33 < jakub> richi: the pattern has (clobber (match_scratch:QI 6)) 11:33 < jakub> richi: and it emits: 11:33 < jakub> ... 11:33 < jakub> rtx operand6 ATTRIBUTE_UNUSED; 11:33 < jakub> ... 11:34 < richi> jakub: and yes, the fixed-value.c is if (len == 0) { ... do nonsense .. } else { .. here we always come ...} ( not sure yet how we arrive there) 11:34 < jakub> operand6 = operands[6]; 11:34 < jakub> (void) operand6; 11:34 < jakub> .. 11:34 < richi> shouldn't that be optimized away? 11:34 < jakub> and then: 11:34 < richi> ah, early uninit runs too early I guess 11:34 < jakub> gen_rtx_SCRATCH (QImode)) (rather than using operand6) 11:35 < richi> yeah, before any optimization ... 11:35 < richi> not so intelligent for the memory stuff And In file included from /space/rguenther/src/svn/trunk/gcc/coretypes.h:365:0, from /space/rguenther/src/svn/trunk/gcc/ipa-cp.c:105: In member function ‘generic_wide_int<storage>& generic_wide_int<T>::operator=(const T&) [with T = wi::hwi_with_prec; storage = wide_int_storage]’, inlined from ‘void ipcp_store_vr_results()’ at /space/rguenther/src/svn/trunk/gcc/ipa-cp.c:4962:49, inlined from ‘unsigned int ipcp_driver()’ at /space/rguenther/src/svn/trunk/gcc/ipa-cp.c:5003:25: /space/rguenther/src/svn/trunk/gcc/wide-int.h:881:3: error: ‘*((void*)&<anonymous> +8)’ may be used uninitialized in this function [-Werror=maybe-uninitialized] storage::operator = (x); ^~~~~~~ vr.min = vr.max = wi::zero (INT_TYPE_SIZE); generic_wide_int <storage>::operator = (const T &x) { storage::operator = (x); can be avoided by removign the assignment (not sure about followup isssues where we look at min/max for VR_VARYING). That seems to be it, leaving the genemit issue. Index: genemit.c =================================================================== --- genemit.c (revision 245601) +++ genemit.c (working copy) @@ -518,6 +518,8 @@ gen_expand (md_rtx_info *info) { for (i = 0; i < stats.num_operand_vars; i++) { + if (i > stats.max_dup_opno && i <= stats.max_scratch_opno) + continue; printf (" operand%d = operands[%d];\n", i, i); printf (" (void) operand%d;\n", i); } results in /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.md: In function ‘rtx_def* gen_tls_global_dynamic_32(rtx, rtx, rtx, rtx)’: /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.md:13535:9: warning: variable ‘operands’ set but not used [-Wunused-but-set-variable] return "call\t{*%p3@GOT(%1)|[DWORD PTR %p3@GOT[%1]]}"; ^ /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.md: In function ‘rtx_def* gen_tls_local_dynamic_base_32(rtx, rtx, rtx)’: /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.md:13655:9: warning: variable ‘operands’ set but not used [-Wunused-but-set-variable] (clobber (match_scratch:SI 4)) ^ we can put ATTRIBUTE_UNUSED on operands. That allows us to build stage2 w/o new warnings...
actually the genemit fix doensn't fully work: /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function ‘rtx_def* gen_smulsi3_highpart(rtx, rtx, rtx)’: /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] (any_extend:TI ~~~~~~ ^ /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function ‘rtx_def* gen_umulsi3_highpart(rtx, rtx, rtx)’: /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] (any_extend:TI ~~~~~~ ^ ... bah.
https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01284.html
Created attachment 40857 [details] unreduced testcase for false positive Bootstrap issue #1 (I posted a reasonable workaround): > ./cc1plus -quiet -O2 -Wuninitialized -o /dev/null ~/fixed-value.ii In file included from /space/rguenther/src/svn/trunk/gcc/coretypes.h:365:0, from /space/rguenther/src/svn/trunk/gcc/fixed-value.c:22: /space/rguenther/src/svn/trunk/gcc/wide-int.h: In function ‘void fixed_from_string(fixed_value*, const char*, machine_mode)’: /space/rguenther/src/svn/trunk/gcc/wide-int.h:818:40: warning: ‘*((void*)& w +34359738360)’ may be used uninitialized in this function [-Wmaybe-uninitialized] unsigned HOST_WIDE_INT high = this->get_val ()[len - 1]; ~~~~~~~~~~~~~~~~^ /space/rguenther/src/svn/trunk/gcc/wide-int.h: In function ‘bool fixed_convert_from_real(fixed_value*, machine_mode, const real_value*, bool)’: /space/rguenther/src/svn/trunk/gcc/wide-int.h:818:40: warning: ‘*((void*)& w +34359738360)’ may be used uninitialized in this function [-Wmaybe-uninitialized] unsigned HOST_WIDE_INT high = this->get_val ()[len - 1]; ~~~~~~~~~~~~~~~~^
Created attachment 40858 [details] unreduced testcase for false positive Bootstrap issue #2 (posted questionable workaround, not analyzed properly): > ./cc1plus -quiet -O2 -Wuninitialized -o /dev/null ~/ipa-cp.ii In file included from /space/rguenther/src/svn/trunk/gcc/coretypes.h:365:0, from /space/rguenther/src/svn/trunk/gcc/ipa-cp.c:105: /space/rguenther/src/svn/trunk/gcc/wide-int.h: In function ‘unsigned int ipcp_driver()’: /space/rguenther/src/svn/trunk/gcc/wide-int.h:881:3: warning: ‘*((void*)&<anonymous> +8)’ may be used uninitialized in this function [-Wmaybe-uninitialized] storage::operator = (x); ^~~~~~~ /space/rguenther/src/svn/trunk/gcc/wide-int.h:881:3: warning: ‘*((void*)&<anonymous> +16)’ may be used uninitialized in this function [-Wmaybe-uninitialized]
(In reply to Richard Biener from comment #9) > actually the genemit fix doensn't fully work: > > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function > ‘rtx_def* gen_smulsi3_highpart(rtx, rtx, rtx)’: > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: > ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] > (any_extend:TI > ~~~~~~ ^ > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function > ‘rtx_def* gen_umulsi3_highpart(rtx, rtx, rtx)’: > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: > ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] > (any_extend:TI > ~~~~~~ ^ > ... > > bah. --- gcc/genemit.c.jj 2017-01-01 12:45:35.000000000 +0100 +++ gcc/genemit.c 2017-03-01 10:51:27.474179940 +0100 @@ -518,6 +518,9 @@ gen_expand (md_rtx_info *info) { for (i = 0; i < stats.num_operand_vars; i++) { + if (i > MAX (stats.max_opno, stats.max_dup_opno) + && i <= stats.max_scratch_opno) + continue; printf (" operand%d = operands[%d];\n", i, i); printf (" (void) operand%d;\n", i); } should fix that. Even if some define_insn doesn't have any match_dups, it can still have normal operands.
On Wed, 1 Mar 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79345 > > --- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #9) > > actually the genemit fix doensn't fully work: > > > > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function > > ‘rtx_def* gen_smulsi3_highpart(rtx, rtx, rtx)’: > > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: > > ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] > > (any_extend:TI > > ~~~~~~ ^ > > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function > > ‘rtx_def* gen_umulsi3_highpart(rtx, rtx, rtx)’: > > /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:26: error: > > ‘operands[3]’ is used uninitialized in this function [-Werror=uninitialized] > > (any_extend:TI > > ~~~~~~ ^ > > ... > > > > bah. > > --- gcc/genemit.c.jj 2017-01-01 12:45:35.000000000 +0100 > +++ gcc/genemit.c 2017-03-01 10:51:27.474179940 +0100 > @@ -518,6 +518,9 @@ gen_expand (md_rtx_info *info) > { > for (i = 0; i < stats.num_operand_vars; i++) > { > + if (i > MAX (stats.max_opno, stats.max_dup_opno) > + && i <= stats.max_scratch_opno) > + continue; > printf (" operand%d = operands[%d];\n", i, i); > printf (" (void) operand%d;\n", i); > } > > should fix that. Even if some define_insn doesn't have any match_dups, it can > still have normal operands. Looks like we have patterns with dups after scratches: (define_expand "<s>mul<mode>3_highpart" [(parallel [(set (match_operand:SWI48 0 "register_operand") (truncate:SWI48 (lshiftrt:<DWI> (mult:<DWI> (any_extend:<DWI> (match_operand:SWI48 1 "nonimmediate_operand")) (any_extend:<DWI> (match_operand:SWI48 2 "register_operand"))) (match_dup 4)))) (clobber (match_scratch:SWI48 3)) (clobber (reg:CC FLAGS_REG))])] which then still warns: /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md: In function ‘rtx_def* gen_umulsi3_highpart(rtx, rtx, rtx)’: /space/rguenther/src/svn/trunk/gcc/config/i386/i386.md:7380:14: warning: ‘operands[3]’ is used uninitialized in this function [-Wuninitialized] (any_extend:TI ~~~~~~^~~~~~~~ ...
Author: jakub Date: Thu Mar 2 09:19:28 2017 New Revision: 245833 URL: https://gcc.gnu.org/viewcvs?rev=245833&root=gcc&view=rev Log: PR tree-optimization/79345 * gensupport.h (struct pattern_stats): Add min_scratch_opno field. * gensupport.c (get_pattern_stats_1) <case MATCH_SCRATCH>: Update it. (get_pattern_stats): Initialize it. * genemit.c (gen_expand): Verify match_scratch numbers come after match_operand/match_dup numbers. * config/i386/i386.md (<s>mul<mode>3_highpart): Swap match_dup and match_scratch numbers. * config/i386/sse.md (avx2_gathersi<mode>, avx2_gatherdi<mode>): Likewise. * config/s390/s390.md (trunctdsd2): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md trunk/gcc/config/i386/sse.md trunk/gcc/config/s390/s390.md trunk/gcc/genemit.c trunk/gcc/gensupport.c trunk/gcc/gensupport.h
Author: rguenth Date: Thu Mar 2 13:42:05 2017 New Revision: 245840 URL: https://gcc.gnu.org/viewcvs?rev=245840&root=gcc&view=rev Log: 2017-03-02 Richard Biener <rguenther@suse.de> PR tree-optimization/79345 PR c++/42000 * tree-ssa-alias.c (walk_aliased_vdefs_1): Take a limit param and abort the walk, returning -1 if it is hit. (walk_aliased_vdefs): Take a limit param and pass it on. * tree-ssa-alias.h (walk_aliased_vdefs): Add a limit param, defaulting to 0 and return a signed int. * tree-ssa-uninit.c (struct check_defs_data): New struct. (check_defs): New helper. (warn_uninitialized_vars): Use walk_aliased_vdefs to warn about uninitialized memory. * fixed-value.c (fixed_from_string): Use ulow/uhigh to avoid bogus uninitialized warning. (fixed_convert_from_real): Likewise. * g++.dg/warn/Wuninitialized-7.C: New testcase. * c-c++-common/ubsan/bounds-2.c: Add -Wno-uninitialized. * gcc.dg/uninit-pr19430-2.c: Add expected warning. Added: trunk/gcc/testsuite/g++.dg/warn/Wuninitialized-7.C Modified: trunk/gcc/ChangeLog trunk/gcc/fixed-value.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/ubsan/bounds-2.c trunk/gcc/testsuite/gcc.dg/uninit-pr19430-2.c trunk/gcc/tree-ssa-alias.c trunk/gcc/tree-ssa-alias.h trunk/gcc/tree-ssa-uninit.c
Fixed on trunk, no plans for backporting.
GCC 6.4 is being released, adjusting target milestone.
(In reply to Richard Biener from comment #17) > Fixed on trunk, no plans for backporting. No plans for backporting means this can be closed then.