public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* More aggressive GCC 12 -Wmaybe-uninitialized when using <functional>
@ 2021-07-22  9:19 Stephan Bergmann
  2021-07-22 10:03 ` Jonathan Wakely
  0 siblings, 1 reply; 4+ messages in thread
From: Stephan Bergmann @ 2021-07-22  9:19 UTC (permalink / raw)
  To: gcc, libstdc++

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.)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: More aggressive GCC 12 -Wmaybe-uninitialized when using <functional>
  2021-07-22  9:19 More aggressive GCC 12 -Wmaybe-uninitialized when using <functional> Stephan Bergmann
@ 2021-07-22 10:03 ` Jonathan Wakely
  2021-07-22 13:36   ` Jonathan Wakely
  2021-07-23  9:58   ` Stephan Bergmann
  0 siblings, 2 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-07-22 10:03 UTC (permalink / raw)
  To: Stephan Bergmann; +Cc: gcc Mailing List, libstdc++

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.

This should fix it:

--- 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()
    {
@@ -250,8 +250,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    typedef bool (*_Manager_type)(_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>
@@ -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>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: More aggressive GCC 12 -Wmaybe-uninitialized when using <functional>
  2021-07-22 10:03 ` Jonathan Wakely
@ 2021-07-22 13:36   ` Jonathan Wakely
  2021-07-23  9:58   ` Stephan Bergmann
  1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-07-22 13:36 UTC (permalink / raw)
  To: Stephan Bergmann; +Cc: gcc Mailing List, libstdc++, gcc Patches

[-- 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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: More aggressive GCC 12 -Wmaybe-uninitialized when using <functional>
  2021-07-22 10:03 ` Jonathan Wakely
  2021-07-22 13:36   ` Jonathan Wakely
@ 2021-07-23  9:58   ` Stephan Bergmann
  1 sibling, 0 replies; 4+ messages in thread
From: Stephan Bergmann @ 2021-07-23  9:58 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc Mailing List, libstdc++

On 22/07/2021 12:03, Jonathan Wakely wrote:
> This should fix it:
[...]

Thanks; it indeed fixed the LibreOffice build for me.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-07-23  9:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22  9:19 More aggressive GCC 12 -Wmaybe-uninitialized when using <functional> Stephan Bergmann
2021-07-22 10:03 ` Jonathan Wakely
2021-07-22 13:36   ` Jonathan Wakely
2021-07-23  9:58   ` Stephan Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).