[Bug c++/109801] New: -Wmaybe-uninitialized warning with -O1 on move constructor

[Bug c++/109801] New: -Wmaybe-uninitialized warning with -O1 on move constructor

[szhong at perforce dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

            Bug ID: 109801
           Summary: -Wmaybe-uninitialized warning with -O1 on move
                    constructor
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: szhong at perforce dot com
  Target Milestone: ---

Created attachment 55038
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55038&action=edit
preprocessed file

-O1 flag is causing the -Wmaybe-uninitialized warning on move constructor.

testcase:

#include <vector>

template <typename T>
class list
{
public:
    list();
    ~list();
};

template <class T>
class table
{
public:
    table()
       : buckets_(64), size_(0) {}
    table(table&& other) {
        std::swap(buckets_, other.buckets_);
        std::swap(size_, other.size_);
    }
    ~table() {}
private:
    std::vector<list<T> > buckets_;
    int size_;
};

int main()
{
    table<int> container(std::move(table<int>()));
    return (0);
}

$ g++ -v -save-temps -O1 -Wmaybe-uninitialized -c table.cpp
Using built-in specs.
COLLECT_GCC=g++
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap
--enable-languages=c,c++,fortran,lto --prefix=/opt/rh/gcc-toolset-12/root/usr
--mandir=/opt/rh/gcc-toolset-12/root/usr/share/man
--infodir=/opt/rh/gcc-toolset-12/root/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
--enable-threads=posix --enable-checking=release --enable-multilib
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-gnu-unique-object --enable-linker-build-id
--with-gcc-major-version-only --enable-libstdcxx-backtrace
--with-linker-hash-style=gnu --enable-plugin --enable-initfini-array
--with-isl=/builddir/build/BUILD/gcc-12.1.1-20220628/obj-x86_64-redhat-linux/isl-install
--enable-offload-targets=nvptx-none --without-cuda-driver
--enable-offload-defaulted --enable-gnu-indirect-function --enable-cet
--with-tune=generic --with-arch_64=x86-64-v2 --with-arch_32=x86-64
--build=x86_64-redhat-linux --with-build-config=bootstrap-lto
--enable-link-serialization=1
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.1.1 20220628 (Red Hat 12.1.1-3) (GCC) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O1' '-Wmaybe-uninitialized' '-c'
'-shared-libgcc' '-mtune=generic' '-march=x86-64-v2'
 /opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/cc1plus -E
-quiet -v -D_GNU_SOURCE table.cpp -mtune=generic -march=x86-64-v2
-Wmaybe-uninitialized -O1 -fpch-preprocess -o table.ii
ignoring nonexistent directory
"/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/include-fixed"
ignoring nonexistent directory
"/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../x86_64-redhat-linux/include"
#include "..." search starts here:
#include <...> search starts here:

/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12

/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/x86_64-redhat-linux

/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/backward
 /opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/include
 /usr/local/include
 /opt/rh/gcc-toolset-12/root/usr/include
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O1' '-Wmaybe-uninitialized' '-c'
'-shared-libgcc' '-mtune=generic' '-march=x86-64-v2'
 /opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/cc1plus
-fpreprocessed table.ii -quiet -dumpbase table.cpp -dumpbase-ext .cpp
-mtune=generic -march=x86-64-v2 -O1 -Wmaybe-uninitialized -version -o table.s
GNU C++17 (GCC) version 12.1.1 20220628 (Red Hat 12.1.1-3)
(x86_64-redhat-linux)
        compiled by GNU C version 12.1.1 20220628 (Red Hat 12.1.1-3), GMP
version 6.2.0, MPFR version 4.1.0-p9, MPC version 1.2.1, isl version
isl-0.18-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++17 (GCC) version 12.1.1 20220628 (Red Hat 12.1.1-3)
(x86_64-redhat-linux)
        compiled by GNU C version 12.1.1 20220628 (Red Hat 12.1.1-3), GMP
version 6.2.0, MPFR version 4.1.0-p9, MPC version 1.2.1, isl version
isl-0.18-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 4fe953bdd6603241811586d9fd73a7cb
In file included from
/opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_pair.h:61,
                 from
/opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_algobase.h:64,
                 from /opt/rh/gcc-toolset-12/root/usr/include/c++/12/vector:60,
                 from table.cpp:1:
In function \u2018std::_Require<std::__not_<std::__is_tuple_like<_Tp> >,
std::is_move_constructible<_Tp>, std::is_move_assignable<_Tp> > std::swap(_Tp&,
_Tp&) [with _Tp = int]\u2019,
    inlined from \u2018table<T>::table(table<T>&&) [with T = int]\u2019 at
table.cpp:19:18,
    inlined from \u2018int main()\u2019 at table.cpp:29:49:
/opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/move.h:204:11: warning:
\u2018container.table<int>::size_\u2019 may be used uninitialized
[-Wmaybe-uninitialized]
  204 |       _Tp __tmp = _GLIBCXX_MOVE(__a);
      |           ^~~~~
table.cpp: In function \u2018int main()\u2019:
table.cpp:29:16: note: \u2018container\u2019 declared here
   29 |     table<int> container(std::move(table<int>()));
      |                ^~~~~~~~~
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O1' '-Wmaybe-uninitialized' '-c'
'-shared-libgcc' '-mtune=generic' '-march=x86-64-v2'
 as -v --64 -o table.o table.s
GNU assembler version 2.38 (x86_64-redhat-linux) using BFD version version
2.38-16.el9
COMPILER_PATH=/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/:/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/:/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/:/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/:/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/
LIBRARY_PATH=/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/:/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O1' '-Wmaybe-uninitialized' '-c'
'-shared-libgcc' '-mtune=generic' '-march=x86-64-v2'

$ cat /etc/os-release
NAME="Red Hat Enterprise Linux"
VERSION="9.1 (Plow)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="9.1"
PLATFORM_ID="platform:el9"
PRETTY_NAME="Red Hat Enterprise Linux 9.1 (Plow)"
ANSI_COLOR="0;31"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:redhat:enterprise_linux:9::baseos"
HOME_URL="https://www.redhat.com/"
DOCUMENTATION_URL="https://access.redhat.com/documentation/red_hat_enterprise_linux/9/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"

REDHAT_BUGZILLA_PRODUCT="Red Hat Enterprise Linux 9"
REDHAT_BUGZILLA_PRODUCT_VERSION=9.1
REDHAT_SUPPORT_PRODUCT="Red Hat Enterprise Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.1"

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note in GCC 13 and above we give an extra warning too:
<source>: In function 'int main()':
<source>:30:49: warning: moving a temporary object prevents copy elision
[-Wpessimizing-move]
   30 |     table<int> container(std::move(table<int>()));
      |                                                 ^
<source>:30:49: note: remove 'std::move' call

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[szhong at perforce dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #2 from Scott Zhong <szhong at perforce dot com> ---
Constructing a container and then moving it to a second container causes this
warning to popup. In this case, container1 is not temporary.

int main()
{
    table<int> container1;
    // (... do something with container1 ...)
    table<int> container2(std::move(container1));
    return (0);
}

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
                 CC|                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I see

<bb 5> [local count: 966367642]:
MEM[(struct __as_base  &)&container] ={v} {CLOBBER};
MEM[(struct _Vector_impl_data *)&container] ={v} {CLOBBER};
MEM[(struct _Vector_impl_data *)&container]._M_start = _42;
MEM[(struct _Vector_impl_data *)&container]._M_finish = __cur_30;
MEM[(struct _Vector_impl_data *)&container]._M_end_of_storage = _37;
MEM[(struct _Vector_impl_data *)&D.26141]._M_start = 0B;
MEM[(struct _Vector_impl_data *)&D.26141]._M_finish = 0B;
MEM[(struct _Vector_impl_data *)&D.26141]._M_end_of_storage = 0B;
_7 = MEM[(type &)&container + 24];  <-- we diagnose this load
MEM[(int &)&container + 24] = 0;
MEM[(int &)&D.26141 + 24] = _7;

that looks like the std::swap (size_, other.size_) going wrong.  Or well,
obviously size_ isn't initialized when

    table(table&& other) {
        std::swap(buckets_, other.buckets_);
        std::swap(size_, other.size_);
    }

is run, only other.size_ is.  So it's entirely your fault I think.

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[szhong at perforce dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #4 from Scott Zhong <szhong at perforce dot com> ---
The move constructor "steals" resources rather than make copies of them, and
leave the argument in some valid but otherwise indeterminate state. It is
reasonable that size_ is not initialized in the context of a move constructor.

If you define body to the constructor and destructor for the class list, the
warning goes away, which has nothing to do with size_ being initialized or not.

template <typename T>
class list
{
public:
    list() {}
    ~list() {}
};

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Scott Zhong from comment #4)
> The move constructor "steals" resources rather than make copies of them, and
> leave the argument in some valid but otherwise indeterminate state. It is
> reasonable that size_ is not initialized in the context of a move
> constructor.
> 
> If you define body to the constructor and destructor for the class list, the
> warning goes away, which has nothing to do with size_ being initialized or
> not.
> 
> template <typename T>
> class list
> {
> public:
>     list() {}
>     ~list() {}
> };

Because if you don't have the define then the calls might clobber some other
memory ...

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
> Am 25.05.2023 um 20:24 schrieb pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801
> 
> --- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> (In reply to Scott Zhong from comment #4)
>> The move constructor "steals" resources rather than make copies of them, and
>> leave the argument in some valid but otherwise indeterminate state. It is
>> reasonable that size_ is not initialized in the context of a move
>> constructor.

Maybe, but you still copy an uninitialized variable so the diagnostic is
correct.

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[szhong at perforce dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #7 from Scott Zhong <szhong at perforce dot com> ---
(In reply to rguenther@suse.de from comment #6)
> > Am 25.05.2023 um 20:24 schrieb pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801
> > 
> > --- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> > (In reply to Scott Zhong from comment #4)
> >> The move constructor "steals" resources rather than make copies of them, and
> >> leave the argument in some valid but otherwise indeterminate state. It is
> >> reasonable that size_ is not initialized in the context of a move
> >> constructor.
> 
> Maybe, but you still copy an uninitialized variable so the diagnostic is
> correct.

If I understand you correctly, the following example should produce an
uninitialized variable diagnostics and the fact that it doesn't means it is a
bug in the compiler?

#include <algorithm>

class table
{
public:
    table()
       : size_(0) {}
    table(table&& other) {
        std::swap(size_, other.size_);
    }
    ~table() {}
private:
    int size_;
};

int main()
{
    table container(std::move(table()));
    return (0);
}

[Bug tree-optimization/109801] -Wmaybe-uninitialized warning with -O1 on move constructor

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109801

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Scott Zhong from comment #7) 
> If I understand you correctly, the following example should produce an
> uninitialized variable diagnostics and the fact that it doesn't means it is
> a bug in the compiler?

No because that is all code that can be considered dead.
If you instead had:
```
#include <algorithm>
class table
{
public:
    table()
       : size_(0) {}
    table(table&& other) {
        std::swap(size_, other.size_);
    }
    ~table();
private:
    int size_;
};
int main()
{
    table container(std::move(table()));
    return (0);
}
```

GCC will warn correctly. as GCC does not know that ~table does not touch size_
at all.