Bug 18968 - [4.0 regression] ICE: tree check: expected ssa_name, have addr_expr in vrp_hash
Summary: [4.0 regression] ICE: tree check: expected ssa_name, have addr_expr in vrp_hash
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.0.0
Assignee: Andrew Pinski
URL:
Keywords: ice-checking, ice-on-valid-code, monitored, patch
Depends on:
Blocks:
 
Reported: 2004-12-13 21:28 UTC by Andreas Schwab
Modified: 2005-02-17 13:38 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2004-12-13 22:17:49


Attachments
Testcase (647 bytes, text/plain)
2004-12-13 21:30 UTC, Andreas Schwab
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schwab 2004-12-13 21:28:14 UTC
$ gcc -S -O COWIntrusiveReferenceTestCases.ii 
COWIntrusiveReferenceTestCases.ii: In member function ‘void 
COWIntrusiveReferenceTestCases::copy_constructor()’: 
COWIntrusiveReferenceTestCases.ii:119: internal compiler error: tree check: 
expected ssa_name, have addr_expr in vrp_hash, at tree-ssa-dom.c:3296
Comment 1 Andreas Schwab 2004-12-13 21:30:02 UTC
Created attachment 7730 [details]
Testcase
Comment 2 Volker Reichelt 2004-12-13 22:17:49 UTC
Confirmed. Here's a reduced testcase:

====================================
struct X
{
  int i;
};

struct Y : virtual X {};
struct Z : Y {};

void foo(X* q) { if (q) q->i++; }

struct A
{
  Z* p;
  A() : p(0) { foo((Y*)p); }
};

A a;
====================================
Comment 3 Andrew Pinski 2004-12-13 22:22:50 UTC
Here is another slightly different reduced testcase:
template<class T> struct F
{
 F(): m_pObj(0) {}
 template<class U> F(F<U> const & rhs): m_pObj(rhs.m_pObj)
 {
   G(m_pObj);
 }
 T * m_pObj;
};
struct Atomic_t {int i;};
inline void G(Atomic_t * p) { p->i++; }
struct X: public virtual Atomic_t {};
struct Y: public X {};
void copy_constructor()
{
  F<Y> py;
  F<X> px1(py);
}
Comment 4 Andrew Pinski 2004-12-13 22:38:48 UTC
This patch fixes the problem but I don't know if it is the correct fix:
Index: tree-ssa-dom.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dom.c,v
retrieving revision 2.80
diff -u -p -r2.80 tree-ssa-dom.c
--- tree-ssa-dom.c      13 Dec 2004 20:12:33 -0000      2.80
+++ tree-ssa-dom.c      13 Dec 2004 22:35:50 -0000
@@ -3265,6 +3265,9 @@ record_range (tree cond, basic_block bb)
       struct vrp_element *element;
       varray_type *vrp_records_p;
       void **slot;
+      
+      if (TREE_CODE (TREE_OPERAND (cond, 0)) != SSA_NAME)
+        return;
 
 
       vrp_hash_elt = xmalloc (sizeof (struct vrp_hash_elt));
Comment 5 Andrew Pinski 2004-12-13 22:53:12 UTC
Though the front-end is at partial fault:
  if (D.1739_9 != 0) goto <L0>; else goto <L1>;


that 0 should be a pointer type, not an integer type so my fix is just a workaround for a C++ front-end 
bug.
Comment 6 Volker Reichelt 2004-12-13 22:54:34 UTC
The regression appeared with Jeff's patch
http://gcc.gnu.org/ml/gcc-cvs/2004-10/msg01710.html
Comment 7 Jeffrey A. Law 2004-12-13 23:00:45 UTC
Subject: Re:  [4.0 regression] ICE: tree
	check: expected ssa_name, have addr_expr in vrp_hash

On Mon, 2004-12-13 at 22:38 +0000, pinskia at gcc dot gnu dot org wrote:
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-12-13 22:38 -------
> This patch fixes the problem but I don't know if it is the correct fix:
> Index: tree-ssa-dom.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dom.c,v
> retrieving revision 2.80
> diff -u -p -r2.80 tree-ssa-dom.c
> --- tree-ssa-dom.c      13 Dec 2004 20:12:33 -0000      2.80
> +++ tree-ssa-dom.c      13 Dec 2004 22:35:50 -0000
> @@ -3265,6 +3265,9 @@ record_range (tree cond, basic_block bb)
>        struct vrp_element *element;
>        varray_type *vrp_records_p;
>        void **slot;
> +      
> +      if (TREE_CODE (TREE_OPERAND (cond, 0)) != SSA_NAME)
> +        return;
>  
> 
>        vrp_hash_elt = xmalloc (sizeof (struct vrp_hash_elt));
So what does the condition look like?  I don't think we should be 
getting into this code with anything other than an SSA_NAME as
the first argument in a COND_EXPR.

An analysis of the problem is a lot more useful to me than a 
patch with no analysis.

jeff

Comment 8 Andrew Pinski 2004-12-13 23:18:18 UTC
(In reply to comment #7)
> Subject: Re:  [4.0 regression] ICE: tree
>         check: expected ssa_name, have addr_expr in vrp_hash
> 
> So what does the condition look like?  I don't think we should be 
> getting into this code with anything other than an SSA_NAME as
> the first argument in a COND_EXPR.
Sorry about that, I was still working through the code of DOM.

The condition looks like &0B->a == 0 which means that the 0 is an integer but other size of the equal 
expression is a pointer.  This means that the types are mismatched by the front-end.
This patch is the correct fix which fixes the problem in the front-end which fixes the mismatched types:
Index: class.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.692
diff -u -p -r1.692 class.c
--- class.c     8 Dec 2004 08:35:33 -0000       1.692
+++ class.c     13 Dec 2004 23:12:29 -0000
@@ -295,8 +295,11 @@ build_base_path (enum tree_code code,
 
   /* Now that we've saved expr, build the real null test.  */
   if (null_test)
-    null_test = fold (build2 (NE_EXPR, boolean_type_node,
-                             expr, integer_zero_node));
+    {
+      tree zero = cp_convert (TREE_TYPE (expr), integer_zero_node);
+      null_test = fold (build2 (NE_EXPR, boolean_type_node,
+                               expr, zero));
+    }
 
   /* If this is a simple base reference, express it as a COMPONENT_REF.  */
   if (code == PLUS_EXPR && !virtual_access



I found where the problem was in the front-end by using this patch which catches mismatched types 
with EQ_EXPR and NE_EXPRs (only, it should be extended for 4.1 to support more expressions):
Index: tree.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/tree.c,v
retrieving revision 1.456
diff -u -p -r1.456 tree.c
--- tree.c      9 Dec 2004 10:54:36 -0000       1.456
+++ tree.c      13 Dec 2004 23:16:53 -0000
@@ -2573,6 +2573,10 @@ build2_stat (enum tree_code code, tree t
 
   t = make_node_stat (code PASS_MEM_STAT);
   TREE_TYPE (t) = tt;
+  if (code == EQ_EXPR || code == NE_EXPR)
+    {
+      gcc_assert (TREE_TYPE (arg0) == TREE_TYPE (arg1));
+    }
 
   /* Below, we automatically set TREE_SIDE_EFFECTS and TREE_READONLY for the
      result based on those same flags for the arguments.  But if the
Comment 9 Andrew Pinski 2004-12-13 23:50:39 UTC
Mine.

Patch here: <http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00983.html>.
Comment 10 Jeffrey A. Law 2004-12-14 00:38:05 UTC
Subject: Re:  [4.0 regression] ICE: tree check: expected
	ssa_name, have addr_expr in vrp_hash

On Mon, 2004-12-13 at 22:53 +0000, pinskia at gcc dot gnu dot org wrote:
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-12-13 22:53 -------
> Though the front-end is at partial fault:
>   if (D.1739_9 != 0) goto <L0>; else goto <L1>;
> 
> 
> that 0 should be a pointer type, not an integer type so my fix is just a workaround for a C++ front-end 
> bug.
Agreed.

Let's take a looksie at what's going on.


  # BLOCK 2
  # PRED: 1 (true)
<L1>:;
  #   a_5 = V_MAY_DEF <a_4>;
  a.p = 0B;
  #   VUSE <a_5>;
  D.1651_6 = a.p;
  D.1652_7 = &D.1651_6->D.1579;
  if (D.1652_7 != 0) goto <L2>; else goto <L3>;
  # SUCC: 3 (true) 4 (false)


We replace a.p on the RHS of the assignment to D.1651_6 with 0B which
results in:

  # BLOCK 2
  # PRED: 1 (true)
<L1>:;
  #   a_5 = V_MAY_DEF <a_4>;
  a.p = 0B;
  #   VUSE <a_5>;
  D.1651_6 = 0B;
  D.1652_7 = &D.1651_6->D.1579;
  if (D.1652_7 != 0) goto <L2>; else goto <L3>;
  # SUCC: 3 (true) 4 (false)

So far so good.

We then replace D.1651_6 on the RHS of hte assignment to D.1652_7 with 
0B which results in:

  # BLOCK 2
  # PRED: 1 (true)
<L1>:;
  #   a_5 = V_MAY_DEF <a_4>;
  a.p = 0B;
  #   VUSE <a_5>;
  D.1651_6 = 0B;
  D.1652_7 = &0->D.1579;
  if (D.1652_7 != 0) goto <L2>; else goto <L3>;
  # SUCC: 3 (true) 4 (false)


So far, so good.  We then propagate &0->D.1579 into the use of D.1652_7
in the IF statement resulting in:

  # BLOCK 2
  # PRED: 1 (true)
<L1>:;
  #   a_5 = V_MAY_DEF <a_4>;
  a.p = 0B;
  #   VUSE <a_5>;
  D.1651_6 = 0B;
  D.1652_7 = &0->D.1579;
  if (&0->D.1579 != 0) goto <L2>; else goto <L3>;
  # SUCC: 3 (true) 4 (false)

Still OK.

The problem is that we consider the condition a range test because the
second argument is an integer type rather than a pointer test.

BTW, can we open a separate PR for the fact that we do not fold
&0->D.1579 down into a constant.  I doubt it matters a whole
lot, but it's rather silly not to fold &<0>->field down to the
byte offset of the field.


Jeff

Comment 11 Jeffrey A. Law 2004-12-14 00:42:34 UTC
Subject: Re:  [4.0 regression] ICE: tree check: expected
	ssa_name, have addr_expr in vrp_hash

On Mon, 2004-12-13 at 23:18 +0000, pinskia at gcc dot gnu dot org wrote:
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-12-13 23:18 -------
> (In reply to comment #7)
> > Subject: Re:  [4.0 regression] ICE: tree
> >         check: expected ssa_name, have addr_expr in vrp_hash
> > 
> > So what does the condition look like?  I don't think we should be 
> > getting into this code with anything other than an SSA_NAME as
> > the first argument in a COND_EXPR.
> Sorry about that, I was still working through the code of DOM.
> 
> The condition looks like &0B->a == 0 which means that the 0 is an integer but other size of the equal 
> expression is a pointer.  This means that the types are mismatched by the front-end.
> This patch is the correct fix which fixes the problem in the front-end which fixes the mismatched types:
> Index: class.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
> retrieving revision 1.692
> diff -u -p -r1.692 class.c
> --- class.c     8 Dec 2004 08:35:33 -0000       1.692
> +++ class.c     13 Dec 2004 23:12:29 -0000
> @@ -295,8 +295,11 @@ build_base_path (enum tree_code code,
>  
>    /* Now that we've saved expr, build the real null test.  */
>    if (null_test)
> -    null_test = fold (build2 (NE_EXPR, boolean_type_node,
> -                             expr, integer_zero_node));
> +    {
> +      tree zero = cp_convert (TREE_TYPE (expr), integer_zero_node);
> +      null_test = fold (build2 (NE_EXPR, boolean_type_node,
> +                               expr, zero));
> +    }
>  
>    /* If this is a simple base reference, express it as a COMPONENT_REF.  */
>    if (code == PLUS_EXPR && !virtual_access
Looks pretty reasonable to me.  Assuming it passes a regression test
consider it pre-approved along with a suitable entry for the testsuite.

Thanks,
jeff


Comment 12 GCC Commits 2004-12-14 02:22:17 UTC
Subject: Bug 18968

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	pinskia@gcc.gnu.org	2004-12-14 02:21:57

Modified files:
	gcc/cp         : ChangeLog class.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: pr18968.C 

Log message:
	2004-12-13  Andrew Pinski  <pinskia@physics.uc.edu>
	
	PR c++/18968
	* g++.dg/opt/pr18968.C: New test.
	
	2004-12-13  Andrew Pinski  <pinskia@physics.uc.edu>
	
	PR c++/18968
	* class.c (build_base_path): Convert the zero constant to the correct
	type when comparing.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4530&r2=1.4531
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/class.c.diff?cvsroot=gcc&r1=1.693&r2=1.694
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4747&r2=1.4748
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/pr18968.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 13 Andrew Pinski 2004-12-14 02:22:43 UTC
Fixed, I already had posted it to gcc-patches so I just replied to that message about your approval.
Comment 14 Grzegorz Jaskiewcz 2005-02-17 13:26:11 UTC
Fixed for that specific testcase, but can't compile kernel today. 
Kernel version 2.6.11-rc4 vanilia, 
gcc (GCC) 4.0.0 20050217 (experimental) 
 
  CC      arch/x86_64/kernel/setup.o 
arch/x86_64/kernel/setup.c: In function 'setup_arch': 
arch/x86_64/kernel/setup.c:453: internal compiler error: tree check: expected 
ssa_name, have addr_expr in vrp_hash, at tree-ssa-dom.c:3310 
Please submit a full bug report, 
with preprocessed source if appropriate. 
See <URL:http://gcc.gnu.org/bugs.html> for instructions. 
make[1]: *** [arch/x86_64/kernel/setup.o] Error 1 
make: *** [arch/x86_64/kernel] Error 2 
 
.... 
#define EBDA_ADDR_POINTER 0x40E 
static void __init reserve_ebda_region(void) 
{ <-here 
        unsigned int addr; 
        /** 
         * there is a real-mode segmented pointer pointing to the 
         * 4K EBDA area at 0x40E 
         */ 
..... 
 
Comment 15 Andrew Pinski 2005-02-17 13:38:16 UTC
(In reply to comment #14)
> Fixed for that specific testcase, but can't compile kernel today. 
This is most likely the same as PR 20009.
Comment 16 Grzegorz Jaskiewcz 2005-02-17 13:46:25 UTC
Subject: Re:  [4.0 regression] ICE: tree check: expected ssa_name,
 have addr_expr in vrp_hash

pinskia at gcc dot gnu dot org wrote:

>------- Additional Comments From pinskia at gcc dot gnu dot org  2005-02-17 13:38 -------
>(In reply to comment #14)
>  
>
>>Fixed for that specific testcase, but can't compile kernel today. 
>>    
>>
>This is most likely the same as PR 20009.
>  
>
ok. I did quick google search and that was the only PR with simmilar 
error message (only line number was different acctualy).