[Patch] Fix ICE at -O3 during IPCP

[Patch] Fix ICE at -O3 during IPCP

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

Hi,

the attached testcase causes an ICE at -O3 on x86 during IPCP:

#0  internal_error (gmsgid=0x98733cb "in %s, at %s:%d")
    at /home/eric/gnat/gnat-head/src/gcc/diagnostic.c:672
#1  0x0865d5c4 in fancy_abort (
    file=0x98e03a0 "/home/eric/gnat/gnat-head/src/gcc/tree.c", line=6199,
    function=0x98e1aa3 "tree_low_cst")
    at /home/eric/gnat/gnat-head/src/gcc/diagnostic.c:728
#2  0x08d2674b in tree_low_cst (t=0x0, pos=1)
    at /home/eric/gnat/gnat-head/src/gcc/tree.c:6199
#3  0x08bd77f9 in build_ref_for_offset_1 (res=0xffffcc68, type=0xf7d96ec4,
    offset=32, exp_type=0xf7d9672c)
    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1246
#4  0x08bd7c9b in build_ref_for_offset (expr=0xffffcc68, type=0xf7d96ec4,
    offset=32, exp_type=0xf7d9672c, allow_ptr=0 '\0')
    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1324
#5  0x08deedbb in ipcp_lattice_from_jfunc (info=0x9f50f54, lat=0xffffccec,
    jfunc=0x9f28cf8) at /home/eric/gnat/gnat-head/src/gcc/ipa-cp.c:333

(gdb) frame 5
#5  0x08deedbb in ipcp_lattice_from_jfunc (info=0x9f50f54, lat=0xffffccec,
    jfunc=0x9f28cf8) at /home/eric/gnat/gnat-head/src/gcc/ipa-cp.c:333
333           ok = build_ref_for_offset (&t, TREE_TYPE (t),
(gdb) p debug_generic_expr(t)
p__data.F

p__data.F has variable size because it contains the field A which originally 
had variable size...  but has no size anymore since it has been cleared in 
free_lang_data_in_decl:

  if (TREE_CODE (decl) == PARM_DECL
      || TREE_CODE (decl) == FIELD_DECL
      || TREE_CODE (decl) == RESULT_DECL)
    {
      tree unit_size = DECL_SIZE_UNIT (decl);
      tree size = DECL_SIZE (decl);
      if ((unit_size && TREE_CODE (unit_size) != INTEGER_CST)
	  || (size && TREE_CODE (size) != INTEGER_CST))
	{
	  DECL_SIZE_UNIT (decl) = NULL_TREE;
	  DECL_SIZE (decl) = NULL_TREE;
	}

so tree_low_cst is called on the NULL pointer

#3  0x08bd77f9 in build_ref_for_offset_1 (res=0xffffcc68, type=0xf7d96ec4,
    offset=32, exp_type=0xf7d9672c)
    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1246
1246                  size = tree_low_cst (DECL_SIZE (fld), 1);
(gdb) frame 2
#2  0x08d2674b in tree_low_cst (t=0x0, pos=1)
    at /home/eric/gnat/gnat-head/src/gcc/tree.c:6199
6199      gcc_assert (host_integerp (t, pos));

and aborts.

Should build_ref_for_offset_1 be prepared to deal with sizeless fields?
It is already prepared to deal with sizeless arrays immediately below:

	case ARRAY_TYPE:
	  tr_size = TYPE_SIZE (TREE_TYPE (type));
	  if (!tr_size || !host_integerp (tr_size, 1))
	    return false;
	  el_size = tree_low_cst (tr_size, 1);

Patch attached, tested on i586-suse-linux, OK for mainline?


2009-10-19  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-sra.c (build_ref_for_offset_1) <RECORD_TYPE>: Skip fields
	without size or size that can be represented as a host integer.


2009-10-19  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/discr21.ad[sb]: New test.
	* gnat.dg/discr21_pkg.ads: New helper.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 579 bytes --]

Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 152970)
+++ tree-sra.c	(working copy)
@@ -1243,7 +1243,10 @@ build_ref_for_offset_1 (tree *res, tree 
 
 	      pos = int_bit_position (fld);
 	      gcc_assert (TREE_CODE (type) == RECORD_TYPE || pos == 0);
-	      size = tree_low_cst (DECL_SIZE (fld), 1);
+	      tr_size = DECL_SIZE (fld);
+	      if (!tr_size || !host_integerp (tr_size, 1))
+		continue;
+	      size = tree_low_cst (tr_size, 1);
 	      if (pos > offset || (pos + size) <= offset)
 		continue;
 

[-- Attachment #3: discr21.adb --]
[-- Type: text/x-adasrc, Size: 584 bytes --]

-- { dg-do compile }
-- { dg-options "-gnatws -O3" }

with Discr21_Pkg; use Discr21_Pkg;

package body Discr21 is

  type Index is new Natural range 0 .. 100;

  type Arr is array (Index range <> ) of Position;

  type Rec(Size : Index := 1) is record
    A : Arr(1 .. Size);
  end record;

  Data : Rec;

  function To_V(pos : Position) return VPosition is
  begin
    return To_Position(pos.x, pos.y, pos.z);
  end;

  procedure Read(Data : Rec) is
    pos : VPosition := To_V (Data.A(1));
  begin
    null;
  end;

  procedure Test is
  begin
    Read (Data);
  end;

end Discr21;

[-- Attachment #4: discr21.ads --]
[-- Type: text/x-adasrc, Size: 52 bytes --]

package Discr21 is

  procedure Test;

end Discr21;

[-- Attachment #5: discr21_pkg.ads --]
[-- Type: text/x-adasrc, Size: 351 bytes --]

package Discr21_Pkg is

  type Position is record
    x,y,z : Float;
  end record;

  type Dim is (Two, Three);

  type VPosition (D: Dim := Three) is record
    x, y : Float;
    case D is
      when Two => null;
      when Three => z : Float;
    end case;
  end record;

  function To_Position (x, y, z : Float) return VPosition;

end Discr21_Pkg;

Re: [Patch] Fix ICE at -O3 during IPCP

[Steven Bosscher]

On Mon, Oct 19, 2009 at 7:34 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>        * tree-sra.c (build_ref_for_offset_1) <RECORD_TYPE>: Skip fields
>        without size or size that can be represented as a host integer.

"cannot be represented" ?

Ciao!
Steven

Re: [Patch] Fix ICE at -O3 during IPCP

[Eric Botcazou]

> >        * tree-sra.c (build_ref_for_offset_1) <RECORD_TYPE>: Skip fields
> >        without size or size that can be represented as a host integer.
>
> "cannot be represented" ?

Yes, "with size that cannot be represented" or "without size that can be 
represented" would probably be clearer.

-- 
Eric Botcazou

Re: [Patch] Fix ICE at -O3 during IPCP

[Richard Guenther]

On Mon, Oct 19, 2009 at 7:34 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the attached testcase causes an ICE at -O3 on x86 during IPCP:
>
> #0  internal_error (gmsgid=0x98733cb "in %s, at %s:%d")
>    at /home/eric/gnat/gnat-head/src/gcc/diagnostic.c:672
> #1  0x0865d5c4 in fancy_abort (
>    file=0x98e03a0 "/home/eric/gnat/gnat-head/src/gcc/tree.c", line=6199,
>    function=0x98e1aa3 "tree_low_cst")
>    at /home/eric/gnat/gnat-head/src/gcc/diagnostic.c:728
> #2  0x08d2674b in tree_low_cst (t=0x0, pos=1)
>    at /home/eric/gnat/gnat-head/src/gcc/tree.c:6199
> #3  0x08bd77f9 in build_ref_for_offset_1 (res=0xffffcc68, type=0xf7d96ec4,
>    offset=32, exp_type=0xf7d9672c)
>    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1246
> #4  0x08bd7c9b in build_ref_for_offset (expr=0xffffcc68, type=0xf7d96ec4,
>    offset=32, exp_type=0xf7d9672c, allow_ptr=0 '\0')
>    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1324
> #5  0x08deedbb in ipcp_lattice_from_jfunc (info=0x9f50f54, lat=0xffffccec,
>    jfunc=0x9f28cf8) at /home/eric/gnat/gnat-head/src/gcc/ipa-cp.c:333
>
> (gdb) frame 5
> #5  0x08deedbb in ipcp_lattice_from_jfunc (info=0x9f50f54, lat=0xffffccec,
>    jfunc=0x9f28cf8) at /home/eric/gnat/gnat-head/src/gcc/ipa-cp.c:333
> 333           ok = build_ref_for_offset (&t, TREE_TYPE (t),
> (gdb) p debug_generic_expr(t)
> p__data.F
>
> p__data.F has variable size because it contains the field A which originally
> had variable size...  but has no size anymore since it has been cleared in
> free_lang_data_in_decl:
>
>  if (TREE_CODE (decl) == PARM_DECL
>      || TREE_CODE (decl) == FIELD_DECL
>      || TREE_CODE (decl) == RESULT_DECL)
>    {
>      tree unit_size = DECL_SIZE_UNIT (decl);
>      tree size = DECL_SIZE (decl);
>      if ((unit_size && TREE_CODE (unit_size) != INTEGER_CST)
>          || (size && TREE_CODE (size) != INTEGER_CST))
>        {
>          DECL_SIZE_UNIT (decl) = NULL_TREE;
>          DECL_SIZE (decl) = NULL_TREE;
>        }
>
> so tree_low_cst is called on the NULL pointer
>
> #3  0x08bd77f9 in build_ref_for_offset_1 (res=0xffffcc68, type=0xf7d96ec4,
>    offset=32, exp_type=0xf7d9672c)
>    at /home/eric/gnat/gnat-head/src/gcc/tree-sra.c:1246
> 1246                  size = tree_low_cst (DECL_SIZE (fld), 1);
> (gdb) frame 2
> #2  0x08d2674b in tree_low_cst (t=0x0, pos=1)
>    at /home/eric/gnat/gnat-head/src/gcc/tree.c:6199
> 6199      gcc_assert (host_integerp (t, pos));
>
> and aborts.
>
> Should build_ref_for_offset_1 be prepared to deal with sizeless fields?
> It is already prepared to deal with sizeless arrays immediately below:
>
>        case ARRAY_TYPE:
>          tr_size = TYPE_SIZE (TREE_TYPE (type));
>          if (!tr_size || !host_integerp (tr_size, 1))
>            return false;
>          el_size = tree_low_cst (tr_size, 1);
>
> Patch attached, tested on i586-suse-linux, OK for mainline?

Ok.

Thanks,
Richard.

>
> 2009-10-19  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-sra.c (build_ref_for_offset_1) <RECORD_TYPE>: Skip fields
>        without size or size that can be represented as a host integer.
>
>
> 2009-10-19  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gnat.dg/discr21.ad[sb]: New test.
>        * gnat.dg/discr21_pkg.ads: New helper.
>
>
> --
> Eric Botcazou
>