Bug 24351 - [4.1 Regression] ICE in do_simple_structure_copy with some C++ code
Summary: [4.1 Regression] ICE in do_simple_structure_copy with some C++ code
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.1.0
: P1 critical
Target Milestone: 4.1.0
Assignee: Not yet assigned to anyone
URL:
Keywords: alias, ice-on-valid-code
: 23949 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-10-13 12:05 UTC by Richard Biener
Modified: 2005-11-03 15:44 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-10-13 15:09:23


Attachments
testcase (1.08 KB, text/plain)
2005-10-13 12:05 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2005-10-13 12:05:11 UTC
/usr/lib/gcc/i586-suse-linux/4.1.0/cc1plus -fpreprocessed test_bind.ii -quiet -dumpbase test_bind.cc -march=i586 -mtune=i686 -auxbase-strip test_bind.o -O2 -Wall -version -fmessage-length=0 -o test_bind.s
test_bind.cc: In function ‘int main()’:
test_bind.cc:62: internal compiler error: Segmentation fault

#0  0x0869fd5b in do_simple_structure_copy (lhs=
      {type = SCALAR, var = 15, offset = 0}, rhs=
      {type = SCALAR, var = 13, offset = 0}, size=64)
    at tree-ssa-structalias.c:2325
#1  0x086a030d in do_structure_copy (lhsop=0x4027dbb8, rhsop=0x40287108)
    at tree-ssa-structalias.c:2495
#2  0x086a1fe8 in find_func_aliases (t=0x4028345c, ai=0x8988a08)
    at tree-ssa-structalias.c:2796
#3  0x086a4d3b in compute_points_to_sets (ai=0x8988a08)
    at tree-ssa-structalias.c:3639
#4  0x0829ba81 in compute_may_aliases () at tree-ssa-alias.c:263
Comment 1 Richard Biener 2005-10-13 12:05:58 UTC
Created attachment 9982 [details]
testcase

Reduced testcase from libsigc++2
Comment 2 Andrew Pinski 2005-10-13 15:09:23 UTC
Confirmed, nice small testcase:
struct adaptor_base {
};
struct bound_argument {
  bound_argument();
};
template <class T_functor> struct adaptor_functor : public adaptor_base {
  explicit adaptor_functor(const T_functor& _A_functor) : functor_(_A_functor) {
  }
  T_functor functor_;
  bound_argument bound_;
};
template <class T_functor> struct adapts : public adaptor_base {
  explicit adapts(const T_functor& _A_functor) : functor_(_A_functor) {
  }
  adaptor_functor<T_functor> functor_;
};
int main() {
  adapts<adapts<int> > a (adapts<int>(1));
}
Comment 3 Richard Biener 2005-10-13 15:48:20 UTC
*** Bug 23949 has been marked as a duplicate of this bug. ***
Comment 4 Andrew Pinski 2005-10-13 23:09:18 UTC
Here is a simplified testcase without templates, just in case you get confused what the structs look in the source:
struct adaptor_base {};
struct bound_argument {
  bound_argument();
};
struct adaptor_functorint : public adaptor_base {};
struct adaptsint : public adaptor_base {
  adaptsint(const int& _A_functor);
  adaptor_functorint functor_;
};
struct adaptor_functor_adaptsint {
  adaptor_functor_adaptsint(const adaptsint& _A_functor) : functor_(_A_functor)
  {}
  adaptsint functor_;
  bound_argument bound_;
};
struct adapts_adaptsint {
  adapts_adaptsint(const adaptsint& _A_functor) : functor_(_A_functor)
  {}
  adaptor_functor_adaptsint functor_;
};
int main() {
  adapts_adaptsint a (adaptsint(1));
}
Comment 5 Richard Biener 2005-10-24 10:46:52 UTC
We are trying to copy 16 bits of

$5 = {id = 8, name = 0x896d7d0 "D.1877", decl = 0x4019ae18, offset = 8, 
  size = 8, fullsize = 16, next = 0x0, node = 8, address_taken = 0, 
  indirect_target = 0, is_artificial_var = 0, is_special_var = 0, 
  is_unknown_size_var = 0, has_union = 0, is_heap_var = 0, 
  solution = 0x8999708, variables = 0x8999718, complex = 0x0}

to

$1 = {id = 6, name = 0x898aaf7 "a", decl = 0x4019a7e8, offset = 8, size = 8, 
  fullsize = 24, next = 0x899ca54, node = 6, address_taken = 1, 
  indirect_target = 0, is_artificial_var = 0, is_special_var = 0, 
  is_unknown_size_var = 0, has_union = 0, is_heap_var = 0, 
  solution = 0x89996c8, variables = 0x89996d8, complex = 0x0}
$2 = {id = 7, name = 0x896d7c4 "a.bound_", decl = 0x4019a7e8, offset = 16, 
  size = 8, fullsize = 24, next = 0x0, node = 7, address_taken = 1, 
  indirect_target = 0, is_artificial_var = 0, is_special_var = 0, 
  is_unknown_size_var = 0, has_union = 0, is_heap_var = 0, 
  solution = 0x89996e8, variables = 0x89996f8, complex = 0x0}

not counting the non-existant 8 bits padding(?)

static void
do_simple_structure_copy (const struct constraint_expr lhs,
                          const struct constraint_expr rhs,
                          const unsigned HOST_WIDE_INT size)
{
  varinfo_t p = get_varinfo (lhs.var);
  unsigned HOST_WIDE_INT pstart, last;
  pstart = p->offset;
  last = p->offset + size;
  for (; p && p->offset < last; p = p->next)


not adding p->offset to last would fix that.  Hmm, too obvious?
Comment 6 Richard Biener 2005-10-24 11:01:44 UTC
Also, while we have the same types for lhs and rhs in do_structure_copy, namely

(gdb) call debug_tree(lhstype)
 <record_type 0x40232c94 adaptsint sizes-gimplified needs-constructing type_1 type_5 HI
    size <integer_cst 0x4018d300 type <integer_type 0x4019f05c bit_size_type> constant invariant 16>
    unit size <integer_cst 0x4018d318 type <integer_type 0x4019f000 unsigned int> constant invariant 2>
    align 8 symtab 0 alias set -1
    fields <field_decl 0x40232f18 functor_
        type <record_type 0x40232bdc adaptor_functorint sizes-gimplified type_1 type_5 QI
            size <integer_cst 0x4018d1f8 constant invariant 8>
            unit size <integer_cst 0x4018d210 constant invariant 1>
            align 8 symtab 0 alias set -1 fields <type_decl 0x40212c30 adaptor_functorint>
            X() X(constX&) this=(X&) n_parents=1 use_template=0 interface-unknown
            pointer_to_this <pointer_type 0x402377e8> chain <type_decl 0x40212bc8 adaptor_functorint>>
        nonlocal decl_3 QI file /suse/rguenther/src/tests/pr24351.C line 8 size <integer_cst 0x4018d1f8 8> unit size <integer_cst 0x4018d210 1>
        align 8 offset_align 128
        offset <integer_cst 0x4018d198 constant invariant 0> bit offset <integer_cst 0x4018d1f8 8> context <record_type 0x40232c94 adaptsint>
        chain <type_decl 0x40212d68 adaptsint type <record_type 0x40232c94 adaptsint>
            nonlocal decl_4 VOID file /suse/rguenther/src/tests/pr24351.C line 6
            align 1 context <record_type 0x40232c94 adaptsint>
           >>
   needs-constructor X(constX&) this=(X&) n_parents=1 use_template=0 interface-unknown
    pointer_to_this <pointer_type 0x40232e60> chain <type_decl 0x40212d00 adaptsint>>

the full sizes from the variable infos for lhs/rhs do not match:

(gdb) print *get_varinfo(lhs.var)
$9 = {id = 6, name = 0x898aaf7 "a", decl = 0x4019a7e8, offset = 8, size = 8, 
  fullsize = 24, next = 0x899ca54, node = 6, address_taken = 1, 
  indirect_target = 0, is_artificial_var = 0, is_special_var = 0, 
  is_unknown_size_var = 0, has_union = 0, is_heap_var = 0, 
  solution = 0x89996c8, variables = 0x89996d8, complex = 0x0}
(gdb) print *get_varinfo(rhs.var)
$10 = {id = 8, name = 0x896d7d0 "D.1877", decl = 0x4019ae18, offset = 8, 
  size = 8, fullsize = 16, next = 0x0, node = 8, address_taken = 0, 
  indirect_target = 0, is_artificial_var = 0, is_special_var = 0, 
  is_unknown_size_var = 0, has_union = 0, is_heap_var = 0, 
  solution = 0x8999708, variables = 0x8999718, complex = 0x0}


This is during the assignment of

(gdb) call debug_generic_expr(t)
#   SFT.2D.1881_20 = V_MAY_DEF <SFT.2D.1881_17>;
#   VUSE <SFT.0D.1879_8>;
aD.1849.functor_D.1824.functor_D.1764 = D.1877

i.e. during the second may-alias pass.

So another workaround may be in do_structure_copy:


      /* The size only really matters insofar as we don't set more or less of
         the variable.  If we hit an unknown size var, the size should be the
         whole darn thing.  */
      if (get_varinfo (rhs.var)->is_unknown_size_var)
        rhssize = ~0;
      else
        rhssize = get_varinfo (rhs.var)->fullsize; /*TREE_INT_CST_LOW (rhstypesize);*/

      if (get_varinfo (lhs.var)->is_unknown_size_var)
        lhssize = ~0;
      else
        lhssize = get_varinfo (lhs.var)->fullsize; /*TREE_INT_CST_LOW (lhstypesize);*/


Oh well.
Comment 7 Richard Biener 2005-10-24 11:55:57 UTC
All of the fixes are wrong.  The only safe thing papering over the problem is
disallowing this case with

Index: tree-ssa-structalias.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-structalias.c,v
retrieving revision 2.32
diff -c -3 -p -r2.32 tree-ssa-structalias.c
*** tree-ssa-structalias.c      6 Oct 2005 21:36:52 -0000       2.32
--- tree-ssa-structalias.c      24 Oct 2005 11:54:56 -0000
*************** check_for_overlaps (VEC (fieldoff_s,heap
*** 3033,3038 ****
--- 3033,3041 ----

    for (i = 0; VEC_iterate (fieldoff_s, fieldstack, i, fo); i++)
      {
+       if (lastoffset == -1
+         && fo->offset != 0)
+       return true;
        if (fo->offset == lastoffset)
        return true;
        lastoffset = fo->offset;
Comment 8 Daniel Berlin 2005-10-24 12:55:43 UTC
Subject: Re:  [4.1 Regression] ICE in
	do_simple_structure_copy with some C++ code

On Mon, 2005-10-24 at 11:55 +0000, rguenth at gcc dot gnu dot org wrote:
> 
> ------- Comment #7 from rguenth at gcc dot gnu dot org  2005-10-24 11:55 -------
> All of the fixes are wrong.  The only safe thing papering over the problem is
> disallowing this case with

If you look, you'll note that while the assignments have the same type,
one is embedded in a structure and the other isn't.

If you stare at that structure it is embedded in, you'll discover the
empty base for one of the classes seems to appear twice, which is where
we get the fullsize of 16.

Thus, we end up with an extra "field" on one side of the assignment.

We reasonably expect that an assignment between two of the same types
will have the same fields on both sides.

This is really another effect of the problem in 24288, AFAICT.  The
empty base for struct adaptor_base appears twice in one of the
structures, even though it's "not really there" twice.

Comment 9 Mark Mitchell 2005-10-31 06:16:53 UTC
Danny, when you refer to PR 24288, do you mean a different PR?  I don't see the relevance of PR 24288, but I do remember discussing this issue with you and Jason.

Anyhow, for the time being, I think it's fair to punish the C++ front-end by disabling this optimization logic there.  The type information we're giving you really isn't right.
Comment 10 Daniel Berlin 2005-10-31 13:53:57 UTC
Subject: Re:  [4.1 Regression] ICE in
	do_simple_structure_copy with some C++ code

On Mon, 2005-10-31 at 06:16 +0000, mmitchel at gcc dot gnu dot org
wrote:
> 
> ------- Comment #9 from mmitchel at gcc dot gnu dot org  2005-10-31 06:16 -------
> Danny, when you refer to PR 24288, do you mean a different PR?  I don't see the
> relevance of PR 24288, but I do remember discussing this issue with you and
> Jason.

I was actually wrong.  This one is due to empty base differences.
The structure on the LHS gets an extra empty base, and it places it in
the padding of one of the types so that we think the LHS and RHS have a
different number of fields, but in actuality they differ only in empty
bases.  We expect that types on the LHS and RHS of a simple structure
assignment have exactly the same number of fields.

I have a patch for this (which is uglier than i'd like, but c'est la
vie)


> 
> 

Comment 11 Richard Biener 2005-10-31 19:17:36 UTC
And that patch doesn't fix the testcase which is attached.
Comment 12 Richard Biener 2005-10-31 19:31:07 UTC
Oh, and this really is a blocker.
Comment 13 Richard Biener 2005-10-31 19:34:00 UTC
Seems it's not my call.
Comment 14 Richard Biener 2005-10-31 19:56:01 UTC
Mark, this looks more like a P1 issue.  It makes all packages that use libsigc++ (part of the gtk--) fail to compile.  It is related to 24288 but the patch there doesn't fix this issue.
Comment 15 Mark Mitchell 2005-11-02 02:04:23 UTC
Showstopper: we're falling over on popular packages on a primary architecture.
Comment 16 Daniel Berlin 2005-11-02 02:30:04 UTC
Subject: Re:  [4.1 Regression] ICE in
	do_simple_structure_copy with some C++ code

On Wed, 2005-11-02 at 02:04 +0000, mmitchel at gcc dot gnu dot org
wrote:
> 
> ------- Comment #15 from mmitchel at gcc dot gnu dot org  2005-11-02 02:04 -------
> Showstopper: we're falling over on popular packages on a primary architecture.
> 


Comment 17 Daniel Berlin 2005-11-02 02:32:59 UTC
Subject: Re:  [4.1 Regression] ICE in
	do_simple_structure_copy with some C++ code

On Tue, 2005-11-01 at 21:29 -0500, Daniel Berlin wrote:
> On Wed, 2005-11-02 at 02:04 +0000, mmitchel at gcc dot gnu dot org
> wrote:
> > 
> > ------- Comment #15 from mmitchel at gcc dot gnu dot org  2005-11-02 02:04 -------
> > Showstopper: we're falling over on popular packages on a primary architecture.
> > 
> 

Just as a status update, their is currently a patch in the hands of
Richard Guenther for testing on distro builds.

It fixes all the testcases (attached and reduced)


Comment 18 Daniel Berlin 2005-11-03 15:39:52 UTC
Subject: Bug 24351

Author: dberlin
Date: Thu Nov  3 15:39:48 2005
New Revision: 106437

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=106437
Log:
2005-11-03  Daniel Berlin  <dberlin@dberlin.org>

	Fix PR tree-optimization/24351

	* tree-ssa-structalias.c (struct variable_info): Add
	collapsed_into.
	(get_varinfo_fc): New function to follow collapsing.
	(new_var_info): Set collapsed_to to NULL.
	(dump_constraint): Follow collapsing.
	(build_constraint_graph): Handle collapsing.
	(do_simple_structure_copy): Return false if something bad
	happened.
	(collapse_rest_of_var): New function.
	(do_structure_copy): Collapse if do_simple_structure_copy returns
	false.


Added:
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr24351-1.C
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr24351-2.C
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr24351-3.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-structalias.c

Comment 19 Daniel Berlin 2005-11-03 15:42:46 UTC
Fixed
Comment 20 Daniel Berlin 2005-11-03 15:44:29 UTC
Fixed