Re: More aggressive GCC 12 -Wmaybe-uninitialized when using <functional>

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4888 bytes --]

On Thu, 22 Jul 2021 at 11:03, Jonathan Wakely wrote:
>
> On Thu, 22 Jul 2021 at 10:23, Stephan Bergmann via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > Compared to GCC 11 (at least gcc-c++-11.1.1-3.fc34.x86_64), recent GCC
> > 12 trunk emits two "unhelpful" -Wmaybe-uninitialized for
> >
> > > $ cat test.cc
> > > #include <functional>
> > > using fn = std::function<void()>;
> > > fn f(fn x) {
> > >     fn a;
> > >     a = x;
> > >     return x;
> > > }
> >
> > > $ ~/gcc/trunk/inst/bin/g++ -c -Wmaybe-uninitialized -O2 test.cc
> > > In file included from ~/gcc/trunk/inst/include/c++/12.0.0/bits/stl_function.h:60,
> > >                  from ~/gcc/trunk/inst/include/c++/12.0.0/functional:49,
> > >                  from test.cc:1:
> > > In function ‘std::_Require<std::__not_<std::__is_tuple_like<_Tp> >, std::is_move_constructible<_Tp>, std::is_move_assignable<_Tp> > std::swap(_Tp&, _Tp&) [with _Tp = void (*)(const std::_Any_data&)]’,
> > >     inlined from ‘void std::function<_Res(_ArgTypes ...)>::swap(std::function<_Res(_ArgTypes ...)>&) [with _Res = void; _ArgTypes = {}]’ at ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:529:11,
> > >     inlined from ‘std::function<_Res(_ArgTypes ...)>& std::function<_Res(_ArgTypes ...)>::operator=(const std::function<_Res(_ArgTypes ...)>&) [with _Res = void; _ArgTypes = {}]’ at ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:442:20,
> > >     inlined from ‘fn f(fn)’ at test.cc:5:9:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/move.h:204:11: warning: ‘<unnamed>.std::function<void()>::_M_invoker’ may be used uninitialized [-Wmaybe-uninitialized]
> > >   204 |       _Tp __tmp = _GLIBCXX_MOVE(__a);
> > >       |           ^~~~~
> > > In file included from ~/gcc/trunk/inst/include/c++/12.0.0/functional:59,
> > >                  from test.cc:1:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h: In function ‘fn f(fn)’:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:442:9: note: ‘<anonymous>’ declared here
> > >   442 |         function(__x).swap(*this);
> > >       |         ^~~~~~~~~~~~~
> > > In file included from ~/gcc/trunk/inst/include/c++/12.0.0/bits/stl_function.h:60,
> > >                  from ~/gcc/trunk/inst/include/c++/12.0.0/functional:49,
> > >                  from test.cc:1:
> > > In function ‘std::_Require<std::__not_<std::__is_tuple_like<_Tp> >, std::is_move_constructible<_Tp>, std::is_move_assignable<_Tp> > std::swap(_Tp&, _Tp&) [with _Tp = std::_Any_data]’,
> > >     inlined from ‘void std::function<_Res(_ArgTypes ...)>::swap(std::function<_Res(_ArgTypes ...)>&) [with _Res = void; _ArgTypes = {}]’ at ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:527:11,
> > >     inlined from ‘std::function<_Res(_ArgTypes ...)>& std::function<_Res(_ArgTypes ...)>::operator=(const std::function<_Res(_ArgTypes ...)>&) [with _Res = void; _ArgTypes = {}]’ at ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:442:20,
> > >     inlined from ‘fn f(fn)’ at test.cc:5:9:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/move.h:204:11: warning: ‘*(std::_Any_data*)((char*)&<unnamed> + offsetof(std::function, std::function<void()>::<unnamed>))’ may be used uninitialized [-Wmaybe-uninitialized]
> > >   204 |       _Tp __tmp = _GLIBCXX_MOVE(__a);
> > >       |           ^~~~~
> > > In file included from ~/gcc/trunk/inst/include/c++/12.0.0/functional:59,
> > >                  from test.cc:1:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h: In function ‘fn f(fn)’:
> > > ~/gcc/trunk/inst/include/c++/12.0.0/bits/std_function.h:442:9: note: ‘<anonymous>’ declared here
> > >   442 |         function(__x).swap(*this);
> > >       |         ^~~~~~~~~~~~~
> >
> > This appears to be an issue with more aggressive -Wmaybe-uninitialized
> > in GCC 12 vs. 11, rather than an issue with changes to <functional> in
> > libstdc++ 12 vs. 11, as effectively the same warnings are emitted when I
> > use GCC 12 with libstdc++ 11 with
> >
> > > $ ~/gcc/trunk/inst/bin/g++ -c -Wmaybe-uninitialized -O2 -nostdinc++ -isystem /usr/include/c++/11 -isystem /usr/include/c++/11/x86_64-redhat-linux test.cc
> >
> > The warnings may technically be correct, and I'm not sure whether this
> > is something that should be addressed in the GCC code emitting the
> > warnings or in the libstdc++ <functional> implementation.
> >
> > (I found this when building LibreOffice with recent GCC 12 trunk.)
>
> The problem is that the _Function_base default constructor is
> user-provided, so when std::function value-initializes its base class,
> that doesn't do zero-init first. It just calls the default ctor, which
> doesn't initialize the _M_fucntor member.

I've pushed the attached patch to trunk, after testing on powerpc64le-linux.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2763 bytes --]

commit c22bcfd2f7dc9bb5ad394720f4a612327dc898ba
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 22 11:57:38 2021

    libstdc++: Initialize all subobjects of std::function
    
    The std::function::swap member swaps each data member unconditionally,
    resulting in -Wmaybe-uninitialized warnings for a default constructed
    object. This happens because the _M_invoker and _M_functor members are
    only initialized if the function has a target.
    
    This change ensures that all subobjects are zero-initialized on
    construction.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/std_function.h (_Function_base): Add
            default member initializers and define constructor as defaulted.
            (function::_M_invoker): Add default member initializer.

diff --git a/libstdc++-v3/include/bits/std_function.h b/libstdc++-v3/include/bits/std_function.h
index 31eba2b822c..c08484465c9 100644
--- a/libstdc++-v3/include/bits/std_function.h
+++ b/libstdc++-v3/include/bits/std_function.h
@@ -237,7 +237,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	{ __functor._M_access<_Functor*>() = new _Functor(std::move(__f)); }
       };
 
-    _Function_base() : _M_manager(nullptr) { }
+    _Function_base() = default;
 
     ~_Function_base()
     {
@@ -247,11 +247,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     bool _M_empty() const { return !_M_manager; }
 
-    typedef bool (*_Manager_type)(_Any_data&, const _Any_data&,
-				  _Manager_operation);
+    using _Manager_type
+      = bool (*)(_Any_data&, const _Any_data&, _Manager_operation);
 
-    _Any_data     _M_functor;
-    _Manager_type _M_manager;
+    _Any_data     _M_functor{};
+    _Manager_type _M_manager{};
   };
 
   template<typename _Signature, typename _Functor>
@@ -261,7 +261,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     class _Function_handler<_Res(_ArgTypes...), _Functor>
     : public _Function_base::_Base_manager<_Functor>
     {
-      typedef _Function_base::_Base_manager<_Functor> _Base;
+      using _Base = _Function_base::_Base_manager<_Functor>;
 
     public:
       static bool
@@ -414,7 +414,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	function(_Functor __f)
 	: _Function_base()
 	{
-	  typedef _Function_handler<_Res(_ArgTypes...), _Functor> _My_handler;
+	  using _My_handler = _Function_handler<_Res(_ArgTypes...), _Functor>;
 
 	  if (_My_handler::_M_not_empty_function(__f))
 	    {
@@ -634,8 +634,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     private:
       using _Invoker_type = _Res (*)(const _Any_data&, _ArgTypes&&...);
-      _Invoker_type _M_invoker;
-  };
+      _Invoker_type _M_invoker = nullptr;
+    };
 
 #if __cpp_deduction_guides >= 201606
   template<typename>