Bug 109434 - [12 Regression] std::optional weird -Wmaybe-uninitialized and behaviour with -O2
Summary: [12 Regression] std::optional weird -Wmaybe-uninitialized and behaviour with -O2
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.2.1
: P2 normal
Target Milestone: 12.3
Assignee: Richard Biener
URL:
Keywords: diagnostic, wrong-code
Depends on:
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2023-04-06 11:32 UTC by Tomáš Pecka
Modified: 2023-04-17 10:47 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 10.1.0, 11.1.0, 12.2.1, 13.0
Known to fail: 12.1.0, 12.2.0
Last reconfirmed: 2023-04-06 00:00:00


Attachments
reproducer (278 bytes, text/x-csrc)
2023-04-06 11:32 UTC, Tomáš Pecka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomáš Pecka 2023-04-06 11:32:10 UTC
Created attachment 54816 [details]
reproducer

Hello,

I've seen a compile warning on a code (see attachment) that looks bogus. 

  In file included from optional.cpp:2:
  In member function ‘constexpr bool std::_Optional_base_impl<_Tp, _Dp>::_M_is_engaged() const [with _Tp = int; _Dp = std::_Optional_base<int, true, true>]’,
      inlined from ‘constexpr bool std::optional<_Tp>::has_value() const [with _Tp = int]’ at /usr/include/c++/12.2.1/optional:988:35,
      inlined from ‘int main()’ at optional.cpp:21:63:
  /usr/include/c++/12.2.1/optional:471:58: warning: ‘*(unsigned char*)((char*)&optInt + offsetof(std::optional<int>,std::optional<int>::<unnamed>.std::_Optional_base<int, true, true>::_M_payload.std::_Optional_payload<int, true, true, true>::<unnamed>.std::_Optional_payload_base<int>::_M_engaged))’ may be used uninitialized [-Wmaybe-uninitialized]
    471 |       { return static_cast<const _Dp*>(this)->_M_payload._M_engaged; }
        |                                                          ^~~~~~~~~~
  optional.cpp: In function ‘int main()’:
  optional.cpp:15:24: note: ‘*(unsigned char*)((char*)&optInt + offsetof(std::optional<int>,std::optional<int>::<unnamed>.std::_Optional_base<int, true,   true>::_M_payload.std::_Optional_payload<int, true, true, true>::<unnamed>.std::_Optional_payload_base<int>::_M_engaged))’ was declared here
     15 |     std::optional<int> optInt;
        |                        ^~~~~~

The warning is present only when compiling with -Wall -O2. When I run the executable, I see unexpected weird output, e.g.

  catch 56
  val=72704.000000

which look like some uninitialized variable is really there. When run under valgrind, I see reports about unitialized variables.

Code seems to be working correctly when compiled with -O1 or lower and the executable seem to be behaving expectedly as well.
Compiling and executing under clang++ works as well regardless of the optimization level. 
I don't have older versions of g++ (< 12) but compiling the attached code on godbolt with g++ 11 and lower does not trigger any warning.

Interesting part is that declaring optDbl as std::optional<int> makes the code behave correctly.
Comment 1 Andrew Pinski 2023-04-06 11:52:08 UTC
DSE2 removes the original store:

  optInt = {};
  optInt = foo ();


  Deleted dead store: optInt = {};

Even though foo can throw ...

Confirmed.
Comment 2 Richard Biener 2023-04-11 12:41:22 UTC
Hmm, we did fix such an issue already .. let me take a look.
Comment 3 Richard Biener 2023-04-11 12:55:48 UTC
So the issue is that clear_bytes_written_by doesn't handle exceptions properly
and that's thru initialize_ao_ref_for_dse.
Comment 4 GCC Commits 2023-04-12 06:49:27 UTC
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:6e3e708dbadaae7b504af7fc4410015624793f02

commit r13-7143-g6e3e708dbadaae7b504af7fc4410015624793f02
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 11 15:06:59 2023 +0200

    tree-optimization/109434 - bogus DSE of throwing call LHS
    
    The byte tracking of call LHS didn't properly handle possibly
    throwing calls correctly which cases bogus DSE and in turn, for the
    testcase a bogus uninit diagnostic and (unreliable) wrong-code.
    
            PR tree-optimization/109434
            * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Properly
            handle possibly throwing calls when processing the LHS
            and may-defs are not OK.
    
            * g++.dg/opt/pr109434.C: New testcase.
Comment 5 Richard Biener 2023-04-12 06:52:38 UTC
Fixed on trunk sofar.
Comment 6 Tomáš Pecka 2023-04-13 07:44:37 UTC
I can confirm that the bug now does not appear in our application code that I simplified into attached code. Tested on commit df7f55cb2ae.
Thank you!
Comment 7 GCC Commits 2023-04-17 10:45:09 UTC
The releases/gcc-12 branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:3d2210dafd872e8470e2a6ae5eea74d2669bc055

commit r12-9415-g3d2210dafd872e8470e2a6ae5eea74d2669bc055
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Apr 11 15:06:59 2023 +0200

    tree-optimization/109434 - bogus DSE of throwing call LHS
    
    The byte tracking of call LHS didn't properly handle possibly
    throwing calls correctly which cases bogus DSE and in turn, for the
    testcase a bogus uninit diagnostic and (unreliable) wrong-code.
    
            PR tree-optimization/109434
            * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Properly
            handle possibly throwing calls when processing the LHS
            and may-defs are not OK.  Add mode to initialize a may-def.
            (dse_optimize_stmt): Query may-defs.
    
            * g++.dg/opt/pr109434.C: New testcase.
Comment 8 Richard Biener 2023-04-17 10:47:15 UTC
Fixed.