[Bug c++/106072] New: Bogus -Wnonnull warning breaks rust bootstrap

[Bug c++/106072] New: Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

            Bug ID: 106072
           Summary: Bogus -Wnonnull warning breaks rust bootstrap
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
                CC: msebor at gcc dot gnu.org
  Target Milestone: ---
            Target: sparc*-sun-solaris2.11

Created attachment 53198
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53198&action=edit
Workaround patch

Bootstrapping the devel/rust/master branch on Solaris/SPARC (both 32 and
64-bit)
breaks like this:

In file included from /vol/gcc/src/git/rust/gcc/rust/parse/rust-parse.h:727,
                 from
/vol/gcc/src/git/rust/gcc/rust/expand/rust-macro-builtins.cc:25:
/vol/gcc/src/git/rust/gcc/rust/parse/rust-parse-impl.h: In member function
'Rust::AST::ClosureParam
Rust::Parser<ManagedTokenSource>::parse_closure_param() [with
ManagedTokenSource = Rust::Lexer]':
/vol/gcc/src/git/rust/gcc/rust/parse/rust-parse-impl.h:9022:70: error: 'this'
pointer is null [-Werror=nonnull]
 9022 |                             std::move (type), std::move (outer_attrs));
      |                 

The error doesn't occur on i386-pc-solaris2.11, amd64-pc-solaris2.11,
i686-pc-linux-gnu, x86_64-pc-linux-gnu.

I've hacked around this with the attached patch.

The preprocessed file is 3.5 MB, unfortunately; currently trying to reduce it
to
something manageable, but this is slow going unfortunately.

[Bug c++/106072] Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

--- Comment #1 from Rainer Orth <ro at gcc dot gnu.org> ---
While creduce produced a 198-byte file from the original 3.5 MB source which
still
shows the warning/error, it is correct in that case.

I'm attaching the original preprocessed source instead.  It shows the error
with

cc1plus -fpreprocessed -quiet -mcpu=v9 -O2 -Wall -Wno-narrowing
-Wno-error=format-diag -Wno-long-long -Wno-variadic-macros
-Wno-overlength-strings -Werror -Wno-unused-parameter -std=c++11

on SPARC.

[Bug c++/106072] Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

--- Comment #2 from Rainer Orth <ro at gcc dot gnu.org> ---
Created attachment 53215
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53215&action=edit
Original preprocessed testcase.

[Bug c++/106072] Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

--- Comment #3 from Rainer Orth <ro at gcc dot gnu.org> ---
At least when building the devel/rust/master branch as of 20221111, the warning
doesn't occur any longer.  I haven't investigated why, though.

[Bug c++/106072] Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

--- Comment #4 from Rainer Orth <ro at gcc dot gnu.org> ---
With rust merged into trunk, the warning resurfaced and broke SPARC bootstrap.

[Bug c++/106072] Bogus -Wnonnull warning breaks rust bootstrap

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|12.0                        |13.0
   Target Milestone|---                         |13.0

--- Comment #5 from Rainer Orth <ro at gcc dot gnu.org> ---
Happens on trunk when bootstrapping with rust included.

[Bug c++/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |build
            Summary|Bogus -Wnonnull warning     |[13 Regression] Bogus
                   |breaks rust bootstrap       |-Wnonnull warning breaks
                   |                            |rust bootstrap
             Target|sparc*-sun-solaris2.11      |sparc*-sun-solaris2.11
                   |                            |aarch64-linux-gnu

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also breaks aarch64-linux-gnu.

[Bug c++/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktkachov at gcc dot gnu.org

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 108084 has been marked as a duplicate of this bug. ***

[Bug c++/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The aarch64-linux-gnu preprocessed source is attached to PR 108084 too.
The same preprocessed source does not show the issue on x86_64-linux-gnu either
.....

[Bug ipa/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |ipa
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
#1  0x0000000001cb4b1c in (anonymous namespace)::pass_post_ipa_warn::execute
(this=0x40ec8a0, fun=0xffffcd8ed910) at
/home/ubuntu/src/upstream-gcc-aarch64/gcc/gcc/tree-ssa-ccp.cc:4683
4683                      if (warning_at (loc, OPT_Wnonnull,
(gdb) l
4678                                    ? EXPR_LOCATION (arg)
4679                                    : gimple_location (stmt));
4680                  auto_diagnostic_group d;
4681                  if (argno == 0)
4682                    {
4683                      if (warning_at (loc, OPT_Wnonnull,
4684                                      "%qs pointer is null", "this")
4685                          && fndecl)
4686                        inform (DECL_SOURCE_LOCATION (fndecl),
4687                                "in a call to non-static member function
%qD",

(gdb) p debug_gimple_stmt(stmt)
# .MEM_32 = VDEF <.MEM_75>
D.1130746 = OBJ_TYPE_REF(_6;(const struct Pattern)0B->6B) (0B);
$6 = void

[Bug ipa/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The code definitely has a check for nullptr on pattern:
```
  // parse pattern (which is required)
  std::unique_ptr<AST::Pattern> pattern = parse_pattern ();
  if (pattern == nullptr)
    {
      // not necessarily an error
      return AST::ClosureParam::create_error ();
    }

  // parse optional type of param
  std::unique_ptr<AST::Type> type = nullptr;
  if (lexer.peek_token ()->get_id () == COLON)
    {
      lexer.skip_token ();

      // parse type, which is now required
      type = parse_type ();
      if (type == nullptr)
        {
          Error error (lexer.peek_token ()->get_locus (),
                       "failed to parse type in closure parameter");
          add_error (std::move (error));

          // skip somewhere?
          return AST::ClosureParam::create_error ();
        }
    }

  return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
                            std::move (type), std::move (outer_attrs));
```
Easy workaround is to add right before AST::CLosureParam:
rust_assert (pattern);

That should enforce the compiler to figure out that the load from pattern
unique_ptr is non-null.

[Bug ipa/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-12-14
             Target|sparc*-sun-solaris2.11      |sparc*-sun-solaris2.11
                   |aarch64-linux-gnu,          |aarch64-linux-gnu,
                   |powerpc-darwin9             |powerpc-darwin9,
                   |                            |aarch64-darwin
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> ---
workaround did not work for me on either powerpc darwin or aarch64 (prototype).
x86_64 and i686 bootstrapped OK (without any workaround).

[Bug ipa/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[iains at gcc dot gnu.org]

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

--- Comment #12 from Iain Sandoe <iains at gcc dot gnu.org> ---
hmm:
  std::unique_ptr<AST::Type> type = nullptr;
  if (lexer.peek_token ()->get_id () == COLON)
    {
      lexer.skip_token ();

      // parse type, which is now required
      type = parse_type ();
      if (type == nullptr)
        {
          Error error (lexer.peek_token ()->get_locus (),
                       "failed to parse type in closure parameter");
          add_error (std::move (error));

          // skip somewhere?
          return AST::ClosureParam::create_error ();
        }
    }

if lexer.peek_token ()->get_id () != COLON, then type *will be* nullptr.
(I don't know the rust impl. at all so cannot tell whether that's syntactically
valid).

[Bug ipa/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
No, that is not the problem.
The problem is solely in the:
  return AST::ClosureParam (std::move (pattern), pattern->get_locus (),
                            std::move (type), std::move (outer_attrs));
line.
pattern is a unique_ptr, std::move (pattern) will:
      _GLIBCXX23_CONSTEXPR
      __uniq_ptr_impl(__uniq_ptr_impl&& __u) noexcept
      : _M_t(std::move(__u._M_t))
      { __u._M_ptr() = nullptr; }
and so once std::move (pattern) is done, pattern is nullptr.  And then it calls
pattern->get_locus (), already on the nullptr.

[Bug rust/106072] [13 Regression] Bogus -Wnonnull warning breaks rust bootstrap

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54084
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54084&action=edit
gcc13-pr106072.patch

Untested fix.

[Bug rust/106072] [13 Regression] -Wnonnull warning breaks rust bootstrap

[tschwinge at gcc dot gnu.org]

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

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://github.com/Rust-GCC
                   |                            |/gccrs/pull/1619
                 CC|                            |tschwinge at gcc dot gnu.org
             Blocks|95507                       |
            Summary|[13 Regression] Bogus       |[13 Regression] -Wnonnull
                   |-Wnonnull warning breaks    |warning breaks rust
                   |rust bootstrap              |bootstrap
           Keywords|diagnostic                  |

--- Comment #15 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
Thanks, Jakub!

So, an actual GCC/Rust issue, not PR95507 "(Wnonnull) - [meta-bug]
bogus/missing -Wnonnull".


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95507
[Bug 95507] [meta-bug] bogus/missing -Wnonnull

[Bug rust/106072] [13 Regression] -Wnonnull warning breaks rust bootstrap

[cvs-commit at gcc dot gnu.org]

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

--- Comment #16 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:95dc11475dac06b5eecd904079de8aa94827a36a

commit r13-4697-g95dc11475dac06b5eecd904079de8aa94827a36a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Dec 14 11:35:22 2022 +0100

    rust: Fix up aarch64-linux bootstrap [PR106072]

    Bootstrap fails on aarch64-linux and some other arches with:
    .../gcc/rust/parse/rust-parse-impl.h: In member function
âRust::AST::ClosureParam
Rust::Parser<ManagedTokenSource>::parse_closure_param() [with
ManagedTokenSource = Rust::Lexer]â:
    .../gcc/rust/parse/rust-parse-impl.h:8916:49: error: âthisâ pointer is
null [-Werror=nonnull]
    The problem is that while say on x86_64-linux the side-effects in the
    arguments are evaluated from last argument to first, on aarch64-linux
    it is the other way around, from first to last.  The C++ I believe even
    in C++17 makes the evaluation of those side-effects unordered
    (indeterminately sequenced with no interleaving), so that is fine.
    But, when the call in return statement is evaluated from first to
    last, std::move (pattern) happens before pattern->get_locus () and
    the former will make pattern (std::unique_ptr) a wrapper object around
    nullptr, so dereferencing it later to call get_locus () on it is invalid.

    2022-12-14  Jakub Jelinek  <jakub@redhat.com>

            PR rust/106072
            * parse/rust-parse-impl.h (parse_closure_param): Store
            pattern->get_locus () in a temporary before std::move (pattern) is
            invoked.

[Bug rust/106072] [13 Regression] -Wnonnull warning breaks rust bootstrap

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed(?)

[Bug rust/106072] [13 Regression] -Wnonnull warning breaks rust bootstrap

[ktkachov at gcc dot gnu.org]

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

--- Comment #18 from ktkachov at gcc dot gnu.org ---
(In reply to Richard Biener from comment #17)
> Fixed(?)

Yes on aarch64, thanks!

[Bug rust/106072] [13 Regression] -Wnonnull warning breaks rust bootstrap

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #19 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #18 from ktkachov at gcc dot gnu.org ---
> (In reply to Richard Biener from comment #17)
>> Fixed(?)
>
> Yes on aarch64, thanks!

Same on sparc.