public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0
@ 2022-01-11 22:18 slyfox at gcc dot gnu.org
  2022-01-11 22:31 ` [Bug c++/103984] " pinskia at gcc dot gnu.org
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: slyfox at gcc dot gnu.org @ 2022-01-11 22:18 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 103984
           Summary: [12 regression] Possible maybe-uninitialized false
                    positive on shaderc-2021.0
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

Noticed new warning on shaderc-2021.0 which uses -Werror. Last week's gcc-12
snapshot did not flag it as a warning. Here is the extracted example:

// $ cat glslc/src/main.cc
#include <string>
#include <vector>
#include <cstring>

struct string_piece {
  const char * begin_ = nullptr;
  const char * end_   = nullptr;
  std::string str() const { return std::string(begin_, end_); }

  string_piece(const char* string) : begin_(string), end_(string) {
    end_ += strlen(string);
  }

  string_piece(const string_piece& other) {
    begin_ = other.begin_;
    end_ = other.end_;
  }
};


struct Z { std::string name; int stage; };
extern int to_stage(string_piece file_name);

int main(int argc, char** argv) {
  std::vector<Z> input_files;
  const std::string current_entry_point_name("main");
  const string_piece arg = argv[0];

  input_files.emplace_back(Z{
      arg.str(), // what is the lifetime of this temporary?
      to_stage(arg), // why does this constructor matter?
  });

  return 0;
}

$ g++-12.0.0 -O1 -Wall -Werror -std=c++11  -o main.cc.o -c glslc/src/main.cc

In file included from /<<NIX>>/gcc-12.0.0/include/c++/12.0.0/string:53,
                 from glslc/src/main.cc:1:
In member function 'std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::pointer std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_M_data()
const [with _CharT = char; _Traits = std::char_traits<char>; _Alloc =
std::allocator<char>]',
    inlined from 'bool std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::_M_is_local() const [with _CharT = char; _Traits =
std::char_traits<char>; _Alloc = std::allocator<char>]' at
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:275:23,
    inlined from 'void std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::_M_dispose() [with _CharT = char; _Traits = std::char_traits<char>;
_Alloc = std::allocator<char>]' at
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:286:18,
    inlined from 'std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::~basic_string() [with _CharT = char; _Traits = std::char_traits<char>;
_Alloc = std::allocator<char>]' at
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:793:19,
    inlined from 'int main(int, char**)' at glslc/src/main.cc:35:1:
/<<NIX>>/gcc-12.0.0/include/c++/12.0.0/bits/basic_string.h:235:28: error:
'<unnamed>.Z::name.std::__cxx11::basic_string<char>::_M_dataplus.std::__cxx11::basic_string<char>::_Alloc_hider::_M_p'
may be used uninitialized [-Werror=maybe-uninitialized]
  235 |       { return _M_dataplus._M_p; }
      |                            ^~~~
glslc/src/main.cc: In function 'int main(int, char**)':
glslc/src/main.cc:32:3: note: '<anonymous>' declared here
   32 |   });
      |   ^
cc1plus: all warnings being treated as errors

$ g++-12.0.0 -v
Using built-in specs.
COLLECT_GCC=/<<NIX>>/gcc-12.0.0/bin/g++
COLLECT_LTO_WRAPPER=/<<NIX>>/gcc-12.0.0/libexec/gcc/x86_64-unknown-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with:
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20220109 (experimental) (GCC)

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
@ 2022-01-11 22:31 ` pinskia at gcc dot gnu.org
  2022-01-11 22:34 ` pinskia at gcc dot gnu.org
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-01-11 22:31 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
           Keywords|                            |diagnostic

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Before GCC 12 we had:
{.name=TARGET_EXPR ...

In GCC 12 we get:
<<< Unknown tree: expr_stmt
              D.31068.name = TARGET_EXPR ....

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
  2022-01-11 22:31 ` [Bug c++/103984] " pinskia at gcc dot gnu.org
@ 2022-01-11 22:34 ` pinskia at gcc dot gnu.org
  2022-01-12  7:39 ` rguenth at gcc dot gnu.org
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-01-11 22:34 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=52320,
                   |                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=66139,
                   |                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=66451

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks related to one of the following PRs:
PR 52320
PR 66139
PR 66451

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
  2022-01-11 22:31 ` [Bug c++/103984] " pinskia at gcc dot gnu.org
  2022-01-11 22:34 ` pinskia at gcc dot gnu.org
@ 2022-01-12  7:39 ` rguenth at gcc dot gnu.org
  2022-01-12  8:29 ` jakub at gcc dot gnu.org
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-01-12  7:39 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
                 CC|                            |jason at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Jason pushed a whole set of changes in this area lately, we'd better understand
what's going on here.

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-01-12  7:39 ` rguenth at gcc dot gnu.org
@ 2022-01-12  8:29 ` jakub at gcc dot gnu.org
  2022-01-26 17:48 ` [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329 jason at gcc dot gnu.org
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-01-12  8:29 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-01-12
     Ever confirmed|0                           |1
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-6329-g4f6bc28fc7dd86bd9e7408cbf28de1e973dd1eda

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-01-12  8:29 ` jakub at gcc dot gnu.org
@ 2022-01-26 17:48 ` jason at gcc dot gnu.org
  2022-01-26 18:35 ` jason at gcc dot gnu.org
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jason at gcc dot gnu.org @ 2022-01-26 17:48 UTC (permalink / raw)
  To: gcc-bugs

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-01-26 17:48 ` [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329 jason at gcc dot gnu.org
@ 2022-01-26 18:35 ` jason at gcc dot gnu.org
  2022-01-26 19:03 ` jason at gcc dot gnu.org
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jason at gcc dot gnu.org @ 2022-01-26 18:35 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Sergei Trofimovich from comment #0)
>   input_files.emplace_back(Z{
>       arg.str(), // what is the lifetime of this temporary?

This prvalue initializes the 'name' member of the Z temporary, there is no
separate string temporary.

>       to_stage(arg), // why does this constructor matter?

My fix for PR66139 fixed EH in aggregate initialization so that if evaluation
of to_stage(arg) throws, we properly destroy the previously initialized 'name'
member.  That's the difference pinskia mentions.

It is surprising that we warn with a user-provided copy constructor and don't
with a defaulted constructor.  It changes the code a bit because it means
string_piece isn't trivially copyable, so we need to create a string_piece
temporary for passing to to_stage instead of passing arg directly by bitwise
copy, but I don't know why that would confuse the optimizers into thinking that
_M_p might be uninitialized.

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

* [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-01-26 18:35 ` jason at gcc dot gnu.org
@ 2022-01-26 19:03 ` jason at gcc dot gnu.org
  2022-01-26 19:25 ` [Bug middle-end/103984] " jason at gcc dot gnu.org
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jason at gcc dot gnu.org @ 2022-01-26 19:03 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #5)
> It is surprising that we warn with a user-provided copy constructor and
> don't with a defaulted constructor.  It changes the code a bit because it
> means string_piece isn't trivially copyable, so we need to create a
> string_piece temporary for passing to to_stage instead of passing arg
> directly by bitwise copy, but I don't know why that would confuse the
> optimizers into thinking that _M_p might be uninitialized.

The diff of the GIMPLE is

@@ -40,10 +39,7 @@
                 D$ = 1;
                 try
                   {
-                    string_piece::string_piece (&D$, &arg);
-                    try
-                      {
-                        _2 = to_stage (&D$);
+                        _2 = to_stage (arg);
                         D$.stage = _2;
                         D$ = 0;
                         try
@@ -62,11 +58,6 @@
                             D$ = {CLOBBER};
                           }
                       }
-                    finally
-                      {
-                        D$ = {CLOBBER};
-                      }
-                  }
                 catch
                   {
                     if (D$ != 0) goto <D$>; else goto <D$>;

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-01-26 19:03 ` jason at gcc dot gnu.org
@ 2022-01-26 19:25 ` jason at gcc dot gnu.org
  2022-03-01 15:04 ` marxin at gcc dot gnu.org
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jason at gcc dot gnu.org @ 2022-01-26 19:25 UTC (permalink / raw)
  To: gcc-bugs

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
          Component|c++                         |middle-end
           Assignee|jason at gcc dot gnu.org           |unassigned at gcc dot gnu.org

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> ---
This doesn't seem to be a C++ issue, unassigning myself.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-01-26 19:25 ` [Bug middle-end/103984] " jason at gcc dot gnu.org
@ 2022-03-01 15:04 ` marxin at gcc dot gnu.org
  2022-03-01 15:26 ` [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd cvs-commit at gcc dot gnu.org
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-03-01 15:04 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
Note I have a different test-case from benchmark package that show the same:

$ cat benchmark.ii
namespace std {
inline namespace __cxx11 {}
struct __new_allocator {
  void deallocate(char *, long) { operator delete(0); }
};
template <typename> using __allocator_base = __new_allocator;
template <typename> struct allocator_traits;
template <typename> struct allocator : __allocator_base<int> {};
template <typename _Tp> struct allocator_traits<allocator<_Tp>> {
  using allocator_type = allocator<_Tp>;
  using pointer = _Tp *;
  using size_type = long;
  static void deallocate(allocator_type __a, pointer __p, size_type __n) {
    __a.deallocate(__p, __n);
  }
};
} // namespace std
struct __alloc_traits : std::allocator_traits<std::allocator<char>> {};
namespace std {
template <class> struct initializer_list {
  int *_M_array;
  unsigned long _M_len;
};
namespace __cxx11 {
struct basic_string {
  struct _Alloc_hider : allocator_traits<allocator<char>> {
    pointer _M_p;
  } _M_dataplus;
  basic_string(const char *, const allocator<char> & = allocator<char>());
  ~basic_string() {
    if (_M_dataplus._M_p) {
      allocator<char> __trans_tmp_1;
      __alloc_traits::deallocate(__trans_tmp_1, 0, 1);
    }
  }
};
} // namespace __cxx11
} // namespace std
struct TestCase {
  std::basic_string name;
  bool error_occurred;
  std::basic_string error_message;
};
int AddCases(std::initializer_list<TestCase>);
int dummy68 = AddCases({{"", true, ""}});

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-03-01 15:04 ` marxin at gcc dot gnu.org
@ 2022-03-01 15:26 ` cvs-commit at gcc dot gnu.org
  2022-03-15 14:22 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-01 15:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ad66b03b3c84786e73e73f09be19977b8f3c4ea3

commit r12-7434-gad66b03b3c84786e73e73f09be19977b8f3c4ea3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Mar 1 09:33:21 2022 +0000

    libstdc++: Fix -Wmaybe-uninitialized false positive [PR103984]

    This fixes a false positive warning seen with LTO:

    12/bits/regex_compiler.tcc:443:32: error: '__last_char._M_char' may be used
uninitialized [-Werror=maybe-uninitialized]

    Given that the std::regex code is not very efficient anyway, the
    overhead of initializing this byte should be minimal.

    libstdc++-v3/ChangeLog:

            PR middle-end/103984
            * include/bits/regex_compiler.h (_BracketMatcher::_M_char): Use
            default member initializer.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2022-03-01 15:26 ` [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd cvs-commit at gcc dot gnu.org
@ 2022-03-15 14:22 ` jakub at gcc dot gnu.org
  2022-03-15 15:06 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 14:22 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
What the C++ FE emits looks just weird to me.  In the gimple dump:
                [pr103984.C:35:1] D.31524.name = string_piece::str
([pr103984.C:35:1] &arg); [return slot optimization]
                [pr103984.C:35:1] D.31544 = 1;
                [pr103984.C:29:27] try
                  {
                    [pr103984.C:31:15] string_piece::string_piece
([pr103984.C:31:15] &D.31523, [pr103984.C:31:16] &arg);
                    try
                      {
                        [pr103984.C:31:15] _2 = to_stage ([pr103984.C:31:15]
&D.31523);
                        [pr103984.C:35:1] D.31524.stage = _2;
                        [pr103984.C:35:1] D.31544 = 0;
                        try
                          {
                            try
                              {
                                [pr103984.C:29:27]
std::vector<Z>::emplace_back<Z> ([pr103984.C:29:27] &input_files,
[pr103984.C:29:28] &D.31524);
                              }
                            finally
                              {
                                [pr103984.C:29:28] Z::~Z ([pr103984.C:29:28]
&D.31524);
                              }
                          }
                        finally
                          {
                            [pr103984.C:29:28] D.31524 = {CLOBBER(eol)};
                          }
                      }
                    finally
                      {
                        [pr103984.C:31:15] D.31523 = {CLOBBER(eol)};
                      }
                  }
                catch
                  {
                    [pr103984.C:35:1] if (D.31544 != 0) goto <D.35853>; else
goto <D.35854>;
                    <D.35853>:
                    [pr103984.C:35:1]
std::__cxx11::basic_string<char>::~basic_string ([pr103984.C:35:1]
&D.31524.name);
                    goto <D.35855>;
                    <D.35854>:
                    <D.35855>:
                  }
Note the D.31524 CLOBBER being added only conditionally (if string_piece ctor
doesn't throw), while if it throws, there is a dtor on its element after that
and
no CLOBBER.  The first spot that initializes part of the D.31524 temporary is
the above first line, so it would make more sense to me to wrap this whole
sequence above into try { ... } finally { [pr103984.C:29:28] D.31524 =
{CLOBBER(eol)}; } and remove the inner try finally that does that.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2022-03-15 14:22 ` jakub at gcc dot gnu.org
@ 2022-03-15 15:06 ` jakub at gcc dot gnu.org
  2022-03-15 15:16 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 15:06 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And it is ehcleanup that sinks the clobber.  Before ehcleanup1 we have:
;;   basic block 16, loop depth 0, maybe hot
;;    prev block 15, next block 17, flags: (NEW, REACHABLE, VISITED)
;;    pred:       15 (EH,EXECUTABLE)
<L7>:
  D.31524 ={v} {CLOBBER(eol)};
  resx 10
;;    succ:       17 (EH,EXECUTABLE)

;;   basic block 17, loop depth 0, maybe hot
;;    prev block 16, next block 18, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 (EH,EXECUTABLE)
;;                16 (EH,EXECUTABLE)
  # _3 = PHI <1(4), 0(16)>
<L8>:
  D.31523 ={v} {CLOBBER(eol)};
  resx 9
;;    succ:       18 (EH,EXECUTABLE)

;;   basic block 18, loop depth 0, maybe hot
;;    prev block 17, next block 19, flags: (NEW, REACHABLE, VISITED)
;;    pred:       17 (EH,EXECUTABLE)
<L9>:
  if (_3 != 0)
    goto <bb 19>; [INV]
  else
    goto <bb 22>; [INV]
;;    succ:       19 (TRUE_VALUE,EXECUTABLE)
;;                22 (FALSE_VALUE,EXECUTABLE)

;;   basic block 19, loop depth 0, maybe hot
;;    prev block 18, next block 20, flags: (NEW, REACHABLE, VISITED)
;;    pred:       18 (TRUE_VALUE,EXECUTABLE)
  _55 = MEM[(const struct basic_string *)&D.31524]._M_dataplus._M_p;

so at this point, if we enter bb 17 from the edge from bb 4, D.31524 isn't
clobbered and the
_55 load is done (and conditional operator delete later), if we enter bb 16
then D.31524 is clobbered and then _3 is 0 and we don't try to delete it again.
But ehcleanup1, probably on the assumption that such conditional cleanups
aren't valid, turns that into:
...
;;   basic block 14, loop depth 0, maybe hot
;;    prev block 13, next block 15, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 (EH,EXECUTABLE)
;;                13 (EH,EXECUTABLE)
  # _3 = PHI <1(4), 0(13)>
<L8>:
  D.31524 ={v} {CLOBBER(eol)};
  D.31523 ={v} {CLOBBER(eol)};
  if (_3 != 0)
    goto <bb 15>; [INV]
  else
    goto <bb 18>; [INV]
;;    succ:       15 (TRUE_VALUE,EXECUTABLE)
;;                18 (FALSE_VALUE,EXECUTABLE)

;;   basic block 15, loop depth 0, maybe hot
;;    prev block 14, next block 16, flags: (NEW, REACHABLE, VISITED)
;;    pred:       14 (TRUE_VALUE,EXECUTABLE)
  _55 = MEM[(const struct basic_string *)&D.31524]._M_dataplus._M_p;
i.e. now D.31524 ={v} {CLOBBER(eol)}; is done no matter which path we enver
that bb from and then uninit pass is right about the warning, because
the _55 = MEM... load reads an uninitialized var.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2022-03-15 15:06 ` jakub at gcc dot gnu.org
@ 2022-03-15 15:16 ` jakub at gcc dot gnu.org
  2022-03-15 15:55 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 15:16 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
In particular, in ehcleanup1 it is the sink_clobbers optimization added for
PR51117.
I think it would be strongly preferrable to fix this in the FE, because if we'd
need to change sink_clobbers, I'd be afraid we'd get significantly worse code
generation with -flifetime-dse than before.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2022-03-15 15:16 ` jakub at gcc dot gnu.org
@ 2022-03-15 15:55 ` jakub at gcc dot gnu.org
  2022-03-15 16:55 ` jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 15:55 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
What the gimplifier sees is:
 <target_expr 0x7fffe8f61a48
    type <record_type 0x7fffe8f0bd20 Z addressable needs-constructing cxx-odr-p
type_4 type_5 type_6 BLK
        size <integer_cst 0x7fffe9dd8840 constant 320>
        unit-size <integer_cst 0x7fffe9dd8888 constant 40>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffe8f0bd20
        fields <function_decl 0x7fffe8dfad00 operator= type <method_type
0x7fffe8a07690>
            addressable used public external autoinline decl_3 QI
pr103984.C:21:8 align:16 warn_if_not_align:0 context <record_type
0x7fffe8f0bd20 Z>
            full-name "Z& Z::operator=(Z&&) noexcept"
            not-really-extern chain <function_decl 0x7fffe8dfac00 operator=>>
context <translation_unit_decl 0x7fffea24c168 pr103984.ii>
        full-name "struct Z"
        needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0
use_template=0 interface-unknown
        pointer_to_this <pointer_type 0x7fffe8f0f690> reference_to_this
<reference_type 0x7fffe8f0f9d8> chain <type_decl 0x7fffe8f0c558 Z>>
    side-effects addressable
    arg:0 <var_decl 0x7fffe8e3f510 D.31524 type <record_type 0x7fffe8f0bd20 Z>
        addressable used ignored read BLK pr103984.C:32:3 size <integer_cst
0x7fffe9dd8840 320> unit-size <integer_cst 0x7fffe9dd8888 40>
        align:64 warn_if_not_align:0 context <function_decl 0x7fffe8f0e000
main>>
    arg:1 <statement_list 0x7fffe8e860c0
        type <void_type 0x7fffea25ff18 void type_6 VOID
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea25ff18
            pointer_to_this <pointer_type 0x7fffea267000>>
        side-effects head 0x7fffe8e73660 tail 0x7fffe8e73780 stmts
0x7fffe8e860e0 0x7fffe8f61af0 0x7fffe8e861a0 0x7fffe8e3b758

        stmt <expr_stmt 0x7fffe8e860e0 type <void_type 0x7fffea25ff18 void>
            side-effects
            arg:0 <init_expr 0x7fffe8e3b708 type <record_type 0x7fffea4010a8
string>
                side-effects
                arg:0 <component_ref 0x7fffe95572d0 type <record_type
0x7fffea4010a8 string>
                    arg:0 <var_decl 0x7fffe8e3f510 D.31524> arg:1 <field_decl
0x7fffe8f0c688 name>>
                arg:1 <target_expr 0x7fffe8f618f8 type <record_type
0x7fffea4010a8 string>
                    side-effects tree_0 arg:0 <var_decl 0x7fffe8e3f360 D.31522>
                    arg:1 <aggr_init_expr 0x7fffe8f61888 type <void_type
0x7fffea25ff18 void>
                        side-effects
                        arg:0 <integer_cst 0x7fffea2611b0 constant 4>
                        arg:1 <addr_expr 0x7fffe8e7c600 type <pointer_type
0x7fffe8f67540>
                            constant arg:0 <function_decl 0x7fffe8f04400 str>>
arg:2 <var_decl 0x7fffe8e3f360 D.31522>
                        arg:3 <addr_expr 0x7fffe8e7c5c0 type <pointer_type
0x7fffe8ed7dc8>
                            readonly arg:0 <var_decl 0x7fffe8e3f120 arg>
                            pr103984.C:30:7 start: pr103984.C:30:7 finish:
pr103984.C:30:9>>
                    arg:2 <call_expr 0x7fffe8f618c0 type <void_type
0x7fffea25ff18 void>
                        side-effects nothrow
                        fn <addr_expr 0x7fffe8e7c6a0 type <pointer_type
0x7fffe95c30a8>
                            constant arg:0 <function_decl 0x7fffe95aa200
__dt_comp >>
                        arg:0 <addr_expr 0x7fffe8e7c660 type <pointer_type
0x7fffe95bf930>
                            arg:0 <var_decl 0x7fffe8e3f360 D.31522>>>
                    pr103984.C:30:14 start: pr103984.C:30:7 finish:
pr103984.C:30:15>>
            pr103984.C:35:1 start: pr103984.C:35:1 finish: pr103984.C:35:1>
        stmt <target_expr 0x7fffe8f61af0 type <boolean_type 0x7fffea25fb28
bool>
            side-effects static arg:0 <var_decl 0x7fffe8e3f5a0 D.31544>
            arg:1 <integer_cst 0x7fffea2612d0 constant 1>
            arg:2 <cond_expr 0x7fffe9557390 type <void_type 0x7fffea25ff18
void>
                side-effects arg:0 <var_decl 0x7fffe8e3f5a0 D.31544>
                arg:1 <call_expr 0x7fffe8f61ab8 type <void_type 0x7fffea25ff18
void>
                    side-effects nothrow
                    fn <addr_expr 0x7fffe8e86180 type <pointer_type
0x7fffe95c30a8>
                        constant arg:0 <function_decl 0x7fffe95aa200 __dt_comp
>>
                    arg:0 <addr_expr 0x7fffe8e86140 type <pointer_type
0x7fffe95bf930>

                        arg:0 <component_ref 0x7fffe8a35300 type <record_type
0x7fffea4010a8 string>
                            arg:0 <var_decl 0x7fffe8e3f510 D.31524> arg:1
<field_decl 0x7fffe8f0c688 name>>>>
                arg:2 <void_cst 0x7fffea252c60 type <void_type 0x7fffea25ff18
void>
                    constant>>
            pr103984.C:35:1 start: pr103984.C:35:1 finish: pr103984.C:35:1>
        stmt <expr_stmt 0x7fffe8e861a0 type <void_type 0x7fffea25ff18 void>
            side-effects
            arg:0 <init_expr 0x7fffe8e3b730 type <integer_type 0x7fffea25f5e8
int>
                side-effects
                arg:0 <component_ref 0x7fffe95573c0 type <integer_type
0x7fffea25f5e8 int>
                    arg:0 <var_decl 0x7fffe8e3f510 D.31524> arg:1 <field_decl
0x7fffe8f0c720 stage>>
                arg:1 <call_expr 0x7fffe8f61968 type <integer_type
0x7fffea25f5e8 int>
                    side-effects
                    fn <addr_expr 0x7fffe8e7c840 type <pointer_type
0x7fffe8f67738>
                        constant arg:0 <function_decl 0x7fffe8f0af00 to_stage>>
                    arg:0 <addr_expr 0x7fffe8e7c820 type <reference_type
0x7fffe8f0bf18>
                        side-effects
                        arg:0 <target_expr 0x7fffe8f61930 type <record_type
0x7fffe8ed7690 string_piece>
                            side-effects tree_0 arg:0 <var_decl 0x7fffe8e3f480
D.31523> arg:1 <aggr_init_expr 0x7fffe8f51f80>>>
                    pr103984.C:31:15 start: pr103984.C:31:7 finish:
pr103984.C:31:19>>
            pr103984.C:35:1 start: pr103984.C:35:1 finish: pr103984.C:35:1>
        stmt <modify_expr 0x7fffe8e3b758 type <boolean_type 0x7fffea25fb28
bool>
            side-effects arg:0 <var_decl 0x7fffe8e3f5a0 D.31544>
            arg:1 <integer_cst 0x7fffea2612a0 constant 0>
            pr103984.C:35:1 start: pr103984.C:35:1 finish: pr103984.C:35:1>>
    arg:2 <call_expr 0x7fffe8f61b28 type <void_type 0x7fffea25ff18 void>
        side-effects nothrow
        fn <addr_expr 0x7fffe8e7cda0 type <pointer_type 0x7fffe8f67e70>
            constant arg:0 <function_decl 0x7fffe8f66b00 __dt_comp >>
        arg:0 <addr_expr 0x7fffe8e7c8a0 type <pointer_type 0x7fffe8f0f690>
            arg:0 <var_decl 0x7fffe8e3f510 D.31524>>>
    pr103984.C:29:28 start: pr103984.C:29:28 finish: pr103984.C:32:3>

i.e. the ~Z (&D.31524) is a cleanup is outer, while the ~string (&D.31524.name)
cleanup is on an TARGET_EXPR inside of it (the one with D.31544 var).
But the inner TARGET_EXPR has CLEANUP_EH_ONLY set on it, while the outer one
(with D.31524 var) doesn't.
Jason, is there any way how to arrange for such cases (with help of the C++ FE,
say with some new flag on something or just pure gimplifier) to defer the
D.31524 clobber only to outside of where the eh only cleanup is handled (that
is on the enclosing CLEANUP_POINT_EXPR, right?)?

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2022-03-15 15:55 ` jakub at gcc dot gnu.org
@ 2022-03-15 16:55 ` jakub at gcc dot gnu.org
  2022-03-15 18:00 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 16:55 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Wonder about:
--- gcc/gimplify.cc.jj  2022-03-04 15:14:53.197812540 +0100
+++ gcc/gimplify.cc     2022-03-15 17:44:45.110734179 +0100
@@ -6997,8 +6997,6 @@ gimplify_target_expr (tree *expr_p, gimp

   if (init)
     {
-      tree cleanup = NULL_TREE;
-
       /* TARGET_EXPR temps aren't part of the enclosing block, so add it
         to the temps list.  Handle also variable length TARGET_EXPRs.  */
       if (!poly_int_tree_p (DECL_SIZE (temp)))
@@ -7019,37 +7017,6 @@ gimplify_target_expr (tree *expr_p, gimp
          gimple_add_tmp_var (temp);
        }

-      /* If TARGET_EXPR_INITIAL is void, then the mere evaluation of the
-        expression is supposed to initialize the slot.  */
-      if (VOID_TYPE_P (TREE_TYPE (init)))
-       ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
-      else
-       {
-         tree init_expr = build2 (INIT_EXPR, void_type_node, temp, init);
-         init = init_expr;
-         ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
-         init = NULL;
-         ggc_free (init_expr);
-       }
-      if (ret == GS_ERROR)
-       {
-         /* PR c++/28266 Make sure this is expanded only once. */
-         TARGET_EXPR_INITIAL (targ) = NULL_TREE;
-         return GS_ERROR;
-       }
-      if (init)
-       gimplify_and_add (init, pre_p);
-
-      /* If needed, push the cleanup for the temp.  */
-      if (TARGET_EXPR_CLEANUP (targ))
-       {
-         if (CLEANUP_EH_ONLY (targ))
-           gimple_push_cleanup (temp, TARGET_EXPR_CLEANUP (targ),
-                                CLEANUP_EH_ONLY (targ), pre_p);
-         else
-           cleanup = TARGET_EXPR_CLEANUP (targ);
-       }
-
       /* Add a clobber for the temporary going out of scope, like
         gimplify_bind_expr.  */
       if (gimplify_ctxp->in_cleanup_point_expr
@@ -7079,8 +7046,32 @@ gimplify_target_expr (tree *expr_p, gimp
                }
            }
        }
-      if (cleanup)
-       gimple_push_cleanup (temp, cleanup, false, pre_p);
+
+      /* If TARGET_EXPR_INITIAL is void, then the mere evaluation of the
+        expression is supposed to initialize the slot.  */
+      if (VOID_TYPE_P (TREE_TYPE (init)))
+       ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
+      else
+       {
+         tree init_expr = build2 (INIT_EXPR, void_type_node, temp, init);
+         init = init_expr;
+         ret = gimplify_expr (&init, pre_p, post_p, is_gimple_stmt, fb_none);
+         init = NULL;
+         ggc_free (init_expr);
+       }
+      if (ret == GS_ERROR)
+       {
+         /* PR c++/28266 Make sure this is expanded only once. */
+         TARGET_EXPR_INITIAL (targ) = NULL_TREE;
+         return GS_ERROR;
+       }
+      if (init)
+       gimplify_and_add (init, pre_p);
+
+      /* If needed, push the cleanup for the temp.  */
+      if (TARGET_EXPR_CLEANUP (targ))
+       gimple_push_cleanup (temp, TARGET_EXPR_CLEANUP (targ),
+                            CLEANUP_EH_ONLY (targ), pre_p);

       /* Only expand this once.  */
       TREE_OPERAND (targ, 3) = init;
i.e. emitting the clobber gimple_push_cleanup for the TARGET_EXPRs before the
gimplification of the TARGET_EXPR_INITIAL instead of after it (and emitting the
WCE earlier means the cleanup is later (as in, outer try/finally).

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2022-03-15 16:55 ` jakub at gcc dot gnu.org
@ 2022-03-15 18:00 ` jakub at gcc dot gnu.org
  2022-03-16  8:47 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-15 18:00 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52632
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52632&action=edit
gcc12-pr103984.patch

Full untested patch.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2022-03-15 18:00 ` jakub at gcc dot gnu.org
@ 2022-03-16  8:47 ` jakub at gcc dot gnu.org
  2022-03-16  8:58 ` jakub at gcc dot gnu.org
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-16  8:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52633
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52633&action=edit
gcc12-pr103984.patch

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2022-03-16  8:47 ` jakub at gcc dot gnu.org
@ 2022-03-16  8:58 ` jakub at gcc dot gnu.org
  2022-03-17  8:26 ` cvs-commit at gcc dot gnu.org
  2022-03-17  8:46 ` jakub at gcc dot gnu.org
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-16  8:58 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Unfortunately the #c15 patch regressed:
+FAIL: g++.dg/opt/pr80032.C  -std=gnu++11 (test for excess errors)
+FAIL: g++.dg/opt/pr80032.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/opt/pr80032.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/opt/pr80032.C  -std=gnu++20 (test for excess errors)
+FAIL: g++.dg/opt/pr80032.C  -std=gnu++2b (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++11 (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++20 (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++2b (test for excess errors)
+FAIL: g++.dg/opt/stack2.C  -std=gnu++98 (test for excess errors)
E.g. on stack2.C, the problem is that needs_to_live_in_memory (temp) is false
before TARGET_EXPR_INITIAL is gimplified, so we don't add a clobber at all in
that case with the first patch and so the stack sharing doesn't happen.
This patch is a new attempt, do the TARGET_EXPR_INITIAL gimplification first,
but into a temporary sequence, then push the cleanups (in the order CLOBBER
first (in the end last), then asan poisioning, then TARGET_EXPR_CLEANUP,
which means the opposite order later on, so first some destructors etc., then
poisioning and finally a clobber).

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2022-03-16  8:58 ` jakub at gcc dot gnu.org
@ 2022-03-17  8:26 ` cvs-commit at gcc dot gnu.org
  2022-03-17  8:46 ` jakub at gcc dot gnu.org
  19 siblings, 0 replies; 21+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-17  8:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7276a18aba41eed65c0cf535ae029e0ceeca6c77

commit r12-7686-g7276a18aba41eed65c0cf535ae029e0ceeca6c77
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 17 09:23:45 2022 +0100

    gimplify: Emit clobbers for TARGET_EXPR_SLOT vars later [PR103984]

    As mentioned in the PR, we emit a bogus uninitialized warning but
    easily could emit wrong-code for it or similar testcases too.
    The bug is that we emit clobber for a TARGET_EXPR_SLOT too early:
              D.2499.e = B::qux (&h); [return slot optimization]
              D.2516 = 1;
              try
                {
                  B::B (&D.2498, &h);
                  try
                    {
                      _2 = baz (&D.2498);
                      D.2499.f = _2;
                      D.2516 = 0;
                      try
                        {
                          try
                            {
                              bar (&D.2499);
                            }
                          finally
                            {
                              C::~C (&D.2499);
                            }
                        }
                      finally
                        {
                          D.2499 = {CLOBBER(eol)};
                        }
                    }
                  finally
                    {
                      D.2498 = {CLOBBER(eol)};
                    }
                }
              catch
                {
                  if (D.2516 != 0) goto <D.2517>; else goto <D.2518>;
                  <D.2517>:
                  A::~A (&D.2499.e);
                  goto <D.2519>;
                  <D.2518>:
                  <D.2519>:
                }
    The CLOBBER for D.2499 is essentially only emitted on the non-exceptional
    path, if B::B or baz throws, then there is no CLOBBER for it but there
    is a conditional destructor A::~A (&D.2499.e).  Now, ehcleanup1
    sink_clobbers optimization assumes that clobbers in the EH cases are
    emitted after last use and so sinks the D.2499 = {CLOBBER(eol)}; later,
    so we then have
      # _3 = PHI <1(3), 0(9)>
    <L2>:
      D.2499 ={v} {CLOBBER(eol)};
      D.2498 ={v} {CLOBBER(eol)};
      if (_3 != 0)
        goto <bb 11>; [INV]
      else
        goto <bb 15>; [INV]

      <bb 11> :
      _35 = D.2499.a;
      if (&D.2499.b != _35)
    where that _35 = D.2499.a comes from inline expansion of the A::~A dtor,
    and that is a load from a clobbered memory.

    Now, what the gimplifier sees in this case is a CLEANUP_POINT_EXPR with
    somewhere inside of it a TARGET_EXPR for D.2499 (with the C::~C (&D.2499)
    cleanup) which in its TARGET_EXPR_INITIAL has another TARGET_EXPR for
    D.2516 bool flag which has CLEANUP_EH_ONLY which performs that conditional
    A::~A (&D.2499.e) call.
    The following patch ensures that CLOBBERs (and asan poisoning) are emitted
    after even those gimple_push_cleanup pushed cleanups from within the
    TARGET_EXPR_INITIAL gimplification (i.e. the last point where the slot
could
    be in theory used).  In my first version of the patch I've done it by just
    moving the
          /* Add a clobber for the temporary going out of scope, like
             gimplify_bind_expr.  */
          if (gimplify_ctxp->in_cleanup_point_expr
              && needs_to_live_in_memory (temp))
            {
    ...
            }
    block earlier in gimplify_target_expr, but that regressed a couple of tests
    where temp is marked TREE_ADDRESSABLE only during (well, very early during
    that) the gimplification of TARGET_EXPR_INITIAL, so we didn't emit e.g. on
    pr80032.C or stack2.C tests any clobbers for the slots and thus stack slot
    reuse wasn't performed.
    So that we don't regress those tests, this patch gimplifies
    TARGET_EXPR_INITIAL as before, but doesn't emit it directly into pre_p,
    emits it into a temporary sequence.  Then emits the CLOBBER cleanup
    into pre_p, then asan poisoning if needed, then appends the
    TARGET_EXPR_INITIAL temporary sequence and finally adds TARGET_EXPR_CLEANUP
    gimple_push_cleanup.  The earlier a GIMPLE_WCE appears in the sequence, the
    outer try/finally or try/catch it is.
    So, with this patch the part of the testcase in gimple dump cited above
    looks instead like:
              try
                {
                  D.2499.e = B::qux (&h); [return slot optimization]
                  D.2516 = 1;
                  try
                    {
                      try
                        {
                          B::B (&D.2498, &h);
                          _2 = baz (&D.2498);
                          D.2499.f = _2;
                          D.2516 = 0;
                          try
                            {
                              bar (&D.2499);
                            }
                          finally
                            {
                              C::~C (&D.2499);
                            }
                        }
                      finally
                        {
                          D.2498 = {CLOBBER(eol)};
                        }
                    }
                  catch
                    {
                      if (D.2516 != 0) goto <D.2517>; else goto <D.2518>;
                      <D.2517>:
                      A::~A (&D.2499.e);
                      goto <D.2519>;
                      <D.2518>:
                      <D.2519>:
                    }
                }
              finally
                {
                  D.2499 = {CLOBBER(eol)};
                }

    2022-03-17  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/103984
            * gimplify.cc (gimplify_target_expr): Gimplify type sizes and
            TARGET_EXPR_INITIAL into a temporary sequence, then push clobbers
            and asan unpoisioning, then append the temporary sequence and
            finally the TARGET_EXPR_CLEANUP clobbers.

            * g++.dg/opt/pr103984.C: New test.

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

* [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd
  2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
                   ` (18 preceding siblings ...)
  2022-03-17  8:26 ` cvs-commit at gcc dot gnu.org
@ 2022-03-17  8:46 ` jakub at gcc dot gnu.org
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-17  8:46 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

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

end of thread, other threads:[~2022-03-17  8:46 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 22:18 [Bug c++/103984] New: [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 slyfox at gcc dot gnu.org
2022-01-11 22:31 ` [Bug c++/103984] " pinskia at gcc dot gnu.org
2022-01-11 22:34 ` pinskia at gcc dot gnu.org
2022-01-12  7:39 ` rguenth at gcc dot gnu.org
2022-01-12  8:29 ` jakub at gcc dot gnu.org
2022-01-26 17:48 ` [Bug c++/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329 jason at gcc dot gnu.org
2022-01-26 18:35 ` jason at gcc dot gnu.org
2022-01-26 19:03 ` jason at gcc dot gnu.org
2022-01-26 19:25 ` [Bug middle-end/103984] " jason at gcc dot gnu.org
2022-03-01 15:04 ` marxin at gcc dot gnu.org
2022-03-01 15:26 ` [Bug middle-end/103984] [12 regression] Possible maybe-uninitialized false positive on shaderc-2021.0 since r12-6329-g4f6bc28fc7dd86bd cvs-commit at gcc dot gnu.org
2022-03-15 14:22 ` jakub at gcc dot gnu.org
2022-03-15 15:06 ` jakub at gcc dot gnu.org
2022-03-15 15:16 ` jakub at gcc dot gnu.org
2022-03-15 15:55 ` jakub at gcc dot gnu.org
2022-03-15 16:55 ` jakub at gcc dot gnu.org
2022-03-15 18:00 ` jakub at gcc dot gnu.org
2022-03-16  8:47 ` jakub at gcc dot gnu.org
2022-03-16  8:58 ` jakub at gcc dot gnu.org
2022-03-17  8:26 ` cvs-commit at gcc dot gnu.org
2022-03-17  8:46 ` jakub at gcc dot gnu.org

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