Created attachment 53125 [details] Test case extracted from original code Commit https://gcc.gnu.org/g:6feb628a706e86eb3f303aff388c74bdb29e7381 "Improve warning suppression for inlined functions [PR98512]. " introduced a regression when using std::optional. Ever since that commit, when building optimized code compiler complains: /home/mijn/workspace/openrct2/src/openrct2/drawing/Drawing.String.cpp: In function ‘void ttf_process_string_literal(OpenRCT2::IContext*, std::string_view)’: /home/mijn/workspace/openrct2/src/openrct2/drawing/Drawing.String.cpp:215:62: error: ‘*(long unsigned int*)((char*)&ttfRunIndex + offsetof(std::optional<long unsigned int>,std::optional<long unsigned int>::<unnamed>.std::_Optional_base<long unsigned int, true, true>::<unnamed>))’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 215 | auto len = it.GetIndex() - ttfRunIndex.value(); | ^ gcc -v: Using built-in specs. COLLECT_GCC=/home/mijn/workspace/gcc-install-master/bin/c++ COLLECT_LTO_WRAPPER=/home/mijn/workspace/gcc-install-master/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: ../gcc/configure --prefix=/home/mijn/workspace/gcc-install-master --enable-languages=c++ Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.0.0 20210702 (experimental) (GCC) Provided an extracted test case for the issue mentioned. Original code: https://github.com/OpenRCT2/OpenRCT2/blob/b0ffa9d28e62b862f68a37788931265cb4528835/src/openrct2/drawing/Drawing.String.cpp#L839 Original bug report for OpenRCT2: https://github.com/OpenRCT2/OpenRCT2/issues/17371 Test case on godbolt: https://godbolt.org/z/6Y3cn1Ks1 c++ -O2 -c -o foo test.cpp -Werror=maybe-uninitialized
Note your example no longer compiles on the trunk of GCC as it requires to add: #include <stdint.h>
# ttfRunIndex_64 = PHI <ttfRunIndex_6(52), [/app/example.cpp:207:27] 0(30)> # ttfRunIndex$8_65 = PHI <ttfRunIndex$8_8(52), [/app/example.cpp:207:27] 0(30)> ... <L17>: [/app/example.cpp:213:13] if (ttfRunIndex$8_65 != 0) goto <bb 22>; [33.00%] else goto <bb 48>; [67.00%] ... <bb 22> [local count: 157594832]: [/app/example.cpp:216:62] len_15 = _20 - ttfRunIndex_64; ... # ttfRunIndex_6 = PHI <ttfRunIndex_14(D)(26), ttfRunIndex_64(48), _20(21), ttfRunIndex_64(49)> # ttfRunIndex$8_8 = PHI <0(26), 0(48), [/app/example.cpp:227:29] 1(21), 1(49)> So if anything this is both a missed optimization and the maybe uninitialized predicates is not doing its job correctly.
GCC 12.2 is being released, retargeting bugs to GCC 12.3.
The predicate analysis has difficulties in handling this case, its like void foo (int val) { int uninit; for (;;) { if (..) { .. = val; (*) val = uninit; } # val = PHI <..> } } and we diagnose (*), figuring the loop PHI of 'val' eventually leads to the PHI that post-dominates the use we want to diagnose. One issue with this is that the predicates provided are from a different iteration than those used so we won't have a chance to compare things meanigfully here. I think the best thing is to avoid diagnosing possibly uninitialized uses in the next iteration of a loop because we have no chance to prove the path is unreachable and thus we'll always warn. At least I think so. I do have a patch.
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:c77fae1ca796d6ea06d5cd437909905c3d3d771c commit r13-2134-gc77fae1ca796d6ea06d5cd437909905c3d3d771c Author: Richard Biener <rguenther@suse.de> Date: Fri Aug 19 14:12:52 2022 +0200 tree-optimization/105937 - avoid uninit diagnostics crossing iterations The following avoids adding PHIs to the worklist for uninit processing if we reach them following backedges. That confuses predicate analysis because it assumes the use is happening in the same iteration as the the definition. For the testcase in the PR the situation is like void foo (int val) { int uninit; # val = PHI <..> (B) for (..) { if (..) { .. = val; (C) val = uninit; } # val = PHI <..> (A) } } and starting from (A) with 'uninit' as argument we arrive at (B) and from there at (C). Predicate analysis then tries to prove the predicate of (B) (not the backedge) can prove that the path from (B) to (C) is unreachable which isn't really what it necessary - that's what we'd need to do when the preheader edge of the loop were the edge with the uninitialized def. So the following makes those cases intentionally false negatives. PR tree-optimization/105937 * tree-ssa-uninit.cc (find_uninit_use): Do not queue PHIs on backedges. (execute_late_warn_uninitialized): Mark backedges. * g++.dg/uninit-pr105937.C: New testcase.
Fixed on trunk.
Will the fix get merged to 12.3 as well? This currently prevents the project I work on from compiling with GCC (https://github.com/OpenRCT2/OpenRCT2/issues/17371)
On Mon, 22 Aug 2022, janisozaur+gcc at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105937 > > --- Comment #7 from janisozaur+gcc at gmail dot com --- > Will the fix get merged to 12.3 as well? This currently prevents the project I > work on from compiling with GCC > (https://github.com/OpenRCT2/OpenRCT2/issues/17371) It depends on the fallout, but yes, I think it's applicable.
The releases/gcc-12 branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:627132c9bf9439bf0ac83bb092182055c1e0f3ab commit r12-8818-g627132c9bf9439bf0ac83bb092182055c1e0f3ab Author: Richard Biener <rguenther@suse.de> Date: Fri Aug 19 14:12:52 2022 +0200 tree-optimization/105937 - avoid uninit diagnostics crossing iterations The following avoids adding PHIs to the worklist for uninit processing if we reach them following backedges. That confuses predicate analysis because it assumes the use is happening in the same iteration as the the definition. For the testcase in the PR the situation is like void foo (int val) { int uninit; # val = PHI <..> (B) for (..) { if (..) { .. = val; (C) val = uninit; } # val = PHI <..> (A) } } and starting from (A) with 'uninit' as argument we arrive at (B) and from there at (C). Predicate analysis then tries to prove the predicate of (B) (not the backedge) can prove that the path from (B) to (C) is unreachable which isn't really what it necessary - that's what we'd need to do when the preheader edge of the loop were the edge with the uninitialized def. So the following makes those cases intentionally false negatives. PR tree-optimization/105937 * tree-ssa-uninit.cc (find_uninit_use): Do not queue PHIs on backedges. (execute_late_warn_uninitialized): Mark backedges. * g++.dg/uninit-pr105937.C: New testcase. (cherry picked from commit c77fae1ca796d6ea06d5cd437909905c3d3d771c)
Fixed.