[Bug c++/111274] New: ice in fixup_blocks_walker with -O1 and -fopenmp

[Bug c++/111274] New: ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

            Bug ID: 111274
           Summary: ice in fixup_blocks_walker with -O1 and -fopenmp
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Created attachment 55831
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55831&action=edit
gzipped C++ source code

The attached C++ code does this with recent gcc trunk:

foundBugs $ /home/dcb38/gcc/results.20230825.asan.ubsan/bin/gcc -c -w -O1
-fopenmp bug956.cc
foundBugs $ /home/dcb38/gcc/results.20230829.asan.ubsan/bin/gcc -c -w -O1
-fopenmp bug956.cc
BipartiteGraphPartialColoring/BipartiteGraphPartialOrdering.cpp: In member
function ‘int
ColPack::BipartiteGraphPartialOrdering::RowSmallestLastOrdering_OMP()’:
BipartiteGraphPartialColoring/BipartiteGraphPartialOrdering.cpp:388:3: internal
compiler error: Segmentation fault
0x1156c49 crash_signal(int)
        ../../trunk.year/gcc/toplev.cc:314
0x978dda tree_check(tree_node*, char const*, int, char const*, tree_code)
        ../../trunk.year/gcc/tree.h:3579
0x978dda fixup_blocks_walker(tree_node**, int*, void*)
        ../../trunk.year/gcc/cp/parser.cc:44487

The bug first seems to appear sometime between g:449ab115dece8ac8
and g:f30d6a48635b5b18.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---
Reduced C++ code seems to be:

struct _Vector_base {
  ~_Vector_base();
};
int ColumnSmallestLastOrdering_OMP_i_MaxNumThreads,
    ColumnSmallestLastOrdering_OMP_i_MaxDegree;
void ColumnSmallestLastOrdering_OMP() {
#pragma omp for
  for (int i = 0; i < ColumnSmallestLastOrdering_OMP_i_MaxNumThreads; i++)
    new _Vector_base[ColumnSmallestLastOrdering_OMP_i_MaxDegree];
}

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
In git, there are 71 commits, so trying g:c28c579f0dd9cd27.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

--- Comment #3 from David Binderman <dcb314 at hotmail dot com> ---
All the OpenMP commits in that range are by Sandra Loosemore
<sandra@codesourcery.com>.

Not sure yet, but a likely candidate so far.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

--- Comment #4 from David Binderman <dcb314 at hotmail dot com> ---
Git range is g:4024ddbe50c2d1cb .. g:87f9b6c2cfd7b829,
so 9 commits left.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

David Binderman <dcb314 at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at codesourcery dot com

--- Comment #5 from David Binderman <dcb314 at hotmail dot com> ---
As expected, one for Sandra. List of commits in the good .. bad range
are

commit 87f9b6c2cfd7b8299b2ab50bf587e864ced43185
commit b7c4a12a9df3170090a431fa4364b97b30b87752
commit 410df0843dbf08280813165bca72cfdaa21c8f23
commit 53891f18f32588d86ba0ec1c5e6206df63be714b
commit 143151ac2013c22e471dd674d02c7dec0798d3bd

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

--- Comment #6 from David Binderman <dcb314 at hotmail dot com> ---
$ git bisect good 143151ac2013c22e
53891f18f32588d86ba0ec1c5e6206df63be714b is the first bad commit
commit 53891f18f32588d86ba0ec1c5e6206df63be714b
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Thu Aug 24 17:35:00 2023 +0000

    OpenMP: C++ support for imperfectly-nested loops

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

sandra at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-09-02
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |sandra at gcc dot gnu.org

--- Comment #7 from sandra at gcc dot gnu.org ---
I'll take a peek.  Thanks for the reduced testcase!

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

--- Comment #8 from sandra at gcc dot gnu.org ---
Created attachment 55832
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55832&action=edit
first attempt at fix

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

--- Comment #9 from sandra at gcc dot gnu.org ---
The problem is that it's tripping over a BIND_EXPR with a null BIND_EXPR_BLOCK.
The attached patch stops the testcase from ICE'ing but hasn't been otherwise
tested yet.

I'm not sure what a null BIND_EXPR_BLOCK actually means, or if it might be a
bug elsewhere that such a thing is being created?  The comments on BIND_EXPR in
tree.def seem to imply that there is always an associated block.  If it's
permissible, maybe the fix ought to pass through the superblock pointer in the
recursive call instead of the null block pointer.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[dcb314 at hotmail dot com]

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

David Binderman <dcb314 at hotmail dot com> changed:

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

--- Comment #10 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to sandra from comment #9)
> I'm not sure what a null BIND_EXPR_BLOCK actually means, or if it might be a
> bug elsewhere that such a thing is being created?  The comments on BIND_EXPR
> in tree.def seem to imply that there is always an associated block.  If it's
> permissible, maybe the fix ought to pass through the superblock pointer in
> the recursive call instead of the null block pointer.

My best guess is to ask Richard. If he doesn't know, he is likely
to know someone who will.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

--- Comment #11 from sandra at gcc dot gnu.org ---
OK, I've been digging around in the code.  do_poplevel() only fills in
BIND_EXPR_BLOCK if stmts_are_full_exprs_p() is true.  I haven't figured out the
control flow that affects the latter predicate or how the vars in a BIND_EXPR
end up associated with some other block, but I understand what a
full-expression is in C++, so I think I can go off and come up with some test
cases to exercise that and figure out what the right thing to do with the tree
coming in to this restructuring is.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

--- Comment #12 from sandra at gcc dot gnu.org ---
Improved and tested patch posted here:

https://gcc.gnu.org/pipermail/gcc-patches/2023-September/629616.html

IIUC the temporaries introduced in non-full-expressions are bound in a block
that encloses the entire full-expression.  The restructuring that moves
intervening code into the loop body operates on whole statements, not
subexpressions, so I convinced myself there should be nothing to do here.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Sandra Loosemore <sandra@gcc.gnu.org>:

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

commit r14-3791-gab4bdad49716eb1c60e22e0e617d5eb56b0bac6f
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Thu Sep 7 16:12:20 2023 +0000

    OpenMP: Fix ICE in fixup_blocks_walker [PR111274]

    This ICE was caused by an invalid assumption that all BIND_EXPRs have
    a non-null BIND_EXPR_BLOCK.  In C++ these do exist and are used for
    temporaries introduced in expressions that are not full-expressions.
    Since they have no block to fix up, the traversal can just ignore
    these tree nodes.

    gcc/cp/ChangeLog
            PR c++/111274
            * parser.cc (fixup_blocks_walker): Check for null BIND_EXPR_BLOCK.

    gcc/testsuite/ChangeLog
            PR c++/111274
            * g++.dg/gomp/pr111274.C: New test case.

[Bug c++/111274] ice in fixup_blocks_walker with -O1 and -fopenmp

[sandra at gcc dot gnu.org]

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

sandra at gcc dot gnu.org changed:

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

--- Comment #14 from sandra at gcc dot gnu.org ---
Should be fixed now.