internal compiler error: in add_phi_arg, at tree-phinodes.c:391 [maverick]

[Bryce Harrington]

Inkscape daily builds for maverick [1] are failing with this gcc internal error.

  CXX display/nr-filter-specularlighting.o
display/nr-filter-slot.cpp: In constructor 'Inkscape::Filters::FilterSlot::FilterSlot(Inkscape::DrawingItem*, Inkscape::DrawingContext*, Inkscape::DrawingContext&, const Inkscape::Filters::FilterUnits&)':
display/nr-filter-slot.cpp:38: internal compiler error: in add_phi_arg, at tree-phinodes.c:391
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.4/README.Bugs> for instructions.
make[4]: *** [display/nr-filter-slot.o] Error 1

[1] https://launchpadlibrarian.net/84246305/buildlog_ubuntu-maverick-i386.inkscape_1%3A0.48%2Bdevel%2B10710%2B24%7Emaverick1_FAILEDTOBUILD.txt.gz

The issue is not present on natty or newer builds. It is a known upstream issue (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45709). Fix was committed as http://gcc.gnu.org/viewcvs?view=revision&revision=164400

[Hjl-tools (hjl-tools)]

[hjl@gnu-35 rrs]$ cat foo.cc
struct foo {
  virtual void bar();
  struct Rect {
    int bottom;
  };
  struct Region {

    static Region subtract(const Rect& lhs, const Rect& rhs)
      {
 Region reg;
 Rect* storage = reg.storage;
   {
     if (lhs.bottom > rhs.bottom)
       storage++;
     reg.count = storage - reg.storage;
   }
 return reg;
      }
    Rect storage[4];
    int count;
  };
  Rect dirtyRegion;
  Rect oldDirtyRegion;
};
void foo::bar()
{
  const Region copyBack(Region::subtract(oldDirtyRegion, dirtyRegion));
}
[hjl@gnu-35 rrs]$ /export/gnu/import/rrs/164143/usr/bin/gcc -S -O foo.cc
foo.cc: In member function ‘virtual void foo::bar()’:
foo.cc:27:70: internal compiler error: in add_phi_arg, at tree-phinodes.c:395
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
[hjl@gnu-35 rrs]$

[Pinskia (pinskia)]

Reduced testcase:
struct Region {
    int storage[4];
    int count;
};
static inline Region subtract(int lhs)
{
  Region reg;
  int* storage = reg.storage;
  if (lhs > 0)
    storage++;
  reg.count = storage - reg.storage;
  return reg;
}
void bar(int a)
{
  const Region copyBack(subtract(a));
}

---- CUT ---
Comes from inlining.

[Pinskia (pinskia)]

Not what is happening is an interaction between the inlining and the return slot optimization and the named value optimization.
Before inlining we have:

  # storage_1 = PHI <&<retval>.storage[0](2), &<retval>.storage[1](3)>
...
  copyBack.1_1 = (struct Region *) &copyBack;
  *copyBack.1_1 ={v} subtract (a_2(D)) [return slot optimization];

--- Cut ----
Since &(*copyBack.1_1).storage[0] is not a constant we get an ICE. Why we don't remove the extra cast to begin is questionable. If we change:
  const Region copyBack(subtract(a));
to
  const Region copyBack = (subtract(a));

We can remove the cast and it works.

[Hjl-tools (hjl-tools)]

It was introduced between revision 127644 and 127649.

[Hjl-tools (hjl-tools)]

Revision 127647:

http://gcc.gnu.org/ml/gcc-cvs/2007-08/msg00541.html

introduced:

[hjl@gnu-26 gcc]$ ./xgcc -B./ -S -O ../../../pr45709.cc
../../../pr45709.cc: In member function ‘virtual void foo::bar()’:
../../../pr45709.cc:27: error: PHI def is not a GIMPLE value
storage_8 = PHI <&copyBack.1_1->storage[0](2), &copyBack.1_1->storage[1](3)>

&copyBack.1_1->storage[0];

../../../pr45709.cc:27: error: PHI def is not a GIMPLE value
storage_8 = PHI <&copyBack.1_1->storage[0](2), &copyBack.1_1->storage[1](3)>

&copyBack.1_1->storage[1];

../../../pr45709.cc:27: error: invalid operand to unary operator
&copyBack.1_1->storage;

../../../pr45709.cc:27: internal compiler error: verify_stmts failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
[hjl@gnu-26 gcc]$

[Hjl-tools (hjl-tools)]

Revision 127647 is the first revision which failed to compile this.

[Hjl-tools (hjl-tools)]

This patch:

http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01459.html

fixes the bug, but caused:

FAIL: g++.dg/conversion/op5.C (test for errors, line 18)
FAIL: g++.dg/conversion/op5.C (test for excess errors)

Now, we get

[hjl@gnu-6 gcc]$ ./xgcc -B./ -S -O /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/conversion/op5.C -ansi -pedantic-errors
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/conversion/op5.C: In function \u2018void foo(const B&)\u2019:
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/conversion/op5.C:18:15: error: conversion from \u2018const B\u2019 to non-scalar type \u2018A\u2019 requested
[hjl@gnu-6 gcc]$

"const" is missing.

[Hjl-tools (hjl-tools)]

A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01461.html

[Rguenth (rguenth)]

We run into

              /* With return slot optimization we can end up with
                 non-gimple (foo *)&this->m, fix that here. */
              if (TREE_CODE (new_arg) != SSA_NAME
                  && TREE_CODE (new_arg) != FUNCTION_DECL
                  && !is_gimple_val (new_arg))
                {
                  gimple_seq stmts = NULL;
                  new_arg = force_gimple_operand (new_arg, &stmts, true, NULL);
                  gsi_insert_seq_on_edge_immediate (new_edge, stmts);

but inserting on an edge that needs splitting, which wrecks new_edge.

Index: tree-inline.c
===================================================================
--- tree-inline.c (revision 164388)
+++ tree-inline.c (working copy)
@@ -2021,8 +2021,11 @@ copy_phis_for_bb (basic_block bb, copy_b
     && !is_gimple_val (new_arg))
   {
     gimple_seq stmts = NULL;
+ basic_block tem;
     new_arg = force_gimple_operand (new_arg, &stmts, true, NULL);
- gsi_insert_seq_on_edge_immediate (new_edge, stmts);
+ tem = gsi_insert_seq_on_edge_immediate (new_edge, stmts);
+ if (tem)
+ new_edge = find_edge (tem, new_bb);
   }
        add_phi_arg (new_phi, new_arg, new_edge,
       gimple_phi_arg_location_from_edge (phi, old_edge));

fixes that.

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 11:38:25 2010
New Revision: 164390

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164390
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Fixup new_edge when
 we splitted it.

 * g++.dg/torture/pr45709.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr45709.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-inline.c

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 11:39:44 2010
New Revision: 164391

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164391
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Fixup new_edge when
 we splitted it.

 * g++.dg/torture/pr45709.C: New testcase.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/torture/pr45709.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-inline.c

[Rguenth (rguenth)]

Stopping here for now - I guess doing immediate insertion still can break
things as if there is another PHI node we'll iterate over the edges
again (but now including split ones) and find_edge (new_edge->src->aux, bb)
will break as new_edge->src is the newly inserted block.

I suppose delaying edge insert commits until after at least this block
is finished is better.

[Rguenth (rguenth)]

Yep - testcase that still ICEs (now w/ a segfault):

struct Region {
    int storage[4];
    int count;
};
static inline Region subtract(int lhs)
{
  Region reg;
  int* storage = reg.storage;
  int* storage2 = reg.storage;
  if (lhs > 0)
    storage++, storage2--;
  reg.count = storage - reg.storage + storage2 - reg.storage;
  return reg;
}
void bar(int a)
{
  const Region copyBack(subtract(a));
}

[Rguenth (rguenth)]

new patch in testing.

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 17:13:04 2010
New Revision: 164397

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164397
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Delay commit of edge
 insertions until after all PHI nodes of the block are processed.

 * g++.dg/torture/pr45709-2.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr45709-2.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 17:16:42 2010
New Revision: 164398

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164398
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Delay commit of edge
 insertions until after all PHI nodes of the block are processed.

 * g++.dg/torture/pr45709-2.C: New testcase.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/torture/pr45709-2.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/tree-inline.c

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 17:23:20 2010
New Revision: 164399

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164399
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Delay commit of edge
 insertions until after all PHI nodes of the block are processed.

 * g++.dg/torture/pr45709.C: New testcase.
 * g++.dg/torture/pr45709-2.C: Likewise.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr45709-2.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr45709.C
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_4-branch/gcc/tree-inline.c

[Rguenth (rguenth)]

Backport for 4.3 pending testing.

[Rguenth (rguenth)]

Subject: Bug 45709

Author: rguenth
Date: Sat Sep 18 18:53:53 2010
New Revision: 164400

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164400
Log:
2010-09-18 Richard Guenther <email address hidden>

 PR tree-optimization/45709
 * tree-inline.c (copy_phis_for_bb): Delay commit of edge
 insertions until after all PHI nodes of the block are processed.

 * g++.dg/torture/pr45709.C: New testcase.
 * g++.dg/torture/pr45709-2.C: Likewise.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/torture/pr45709-2.C
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/torture/pr45709.C
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_3-branch/gcc/tree-inline.c

[Rguenth (rguenth)]

Fixed.

[Bryce Harrington (bryce)]

Patch needs pulled and built against the ubuntu gcc-4.4, then ensure tests still past and verify various packages still build properly. Then an SRU can be filed.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Cafuego (cafuego)]

This issue also affects Lucid users (on i3886 only, amd64 builds fine) who try to build Inkscape with a backported cairo and pximan.

[Cafuego (cafuego)]

... as per https://launchpad.net/~cafuego/+archive/inkscape/+build/2926134/+files/buildlog_ubuntu-lucid-i386.inkscape_0.49~devel%2B10740%2B6~lucid1_FAILEDTOBUILD.txt.gz

[Matthias Klose (doko)]

please add the ubuntu-toolchain-r/ppa PPA to your PPA, and recheck the build. I would like to avoid an update in the distribution if the failure is not seen with the inkscape version in maverick.

[Cafuego (cafuego)]

I actually ported the patch to gcc for Lucid and Maverick, then added a PPA containing these compilers (ppa://cafuego/lucid-gcc) and used them to successfully build Inkscape on Lucid and Maverick i386.

[Cafuego (cafuego)]

... and this one.

[Cafuego (cafuego)]

The bundled Inkscape versions build fine on these respective Ubuntu versions, this compiler problem was only introduced earlier this year, so is only triggered when building a much newer Inkscape.

I agree that pushing new a GCC to Lucid and Maverick is probably not needed or wanted :-)

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "lucid-gcc-tree-inline-delay-commit.diff" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

[Alex Valavanis (valavanisalex)]

@Matthias - I have added ppa:ubuntu-toolchain-r/ppa as a dependency of ppa:inkscape.dev/trunk and the package now builds successfully in Maverick.

[Bryce Harrington (bryce)]

Ok, given that pushing a new GCC to lucid/maverick for this issue sounds unwarranted, and the toolchain dependency solves it for the inkscape project itself, shall this bug report be closed out at this time?

Any other actual issues need to be investigated here? If not, I'm fine to call this issue resolved now and close all the tasks.

[Alex Valavanis (valavanisalex)]

Hi Bryce,

I'm not sure if it's really resolved in Ubuntu Maverick - the package would presumably still FTBFS for any user who tried a local build. However, pushing a new version of GCC is understandably a very big deal. Probably best closing as "Won't fix" rather than "Fix Released" so that users at least know that the bug still exists and can see the reason for not pushing the change.

[Alex Valavanis (valavanisalex)]

Marking as fixed in Inkscape - the package builds with updated toolchain dependency in PPA.