[Bug c++/30017] New: ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[Bug c++/30017] New: ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

Between r119279 and r119305 DVL failed to build with

./cc1plus -quiet -o /dev/null
/space/rguenther/src/c++bench/html/sandbox/DLV/dl.ii -O
dl.C: In function 'void printQueryI(std::ostream&, const INTERPRET&, const
GINTERPRET*)':
dl.C:800: internal compiler error: in cp_expr_size, at cp/cp-objcp-common.c:101
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.


-- 
           Summary: ICE in cp_expr_size, at cp/cp-objcp-common.c:101
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2006-11-29 16:33 -------
Reducing.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2006-11-29 17:55 -------
Created an attachment (id=12706)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12706&action=view)
reduced testcase


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2006-11-29 18:46 -------
doesn't fail with 32bits.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|                            |x86_64-*-*


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2006-11-29 19:47 -------
Further reduced testcase:

class NAMES_ITEM { };
struct ATOM {
    const NAMES_ITEM& getPredItem() const { }
};
class ATOMSET {
public:
    class FIND_RESULT {
        const NAMES_ITEM &pattern;
    public:
        FIND_RESULT() : pattern(pattern)  { }
        FIND_RESULT(const FIND_RESULT &f) : pattern(f.pattern) { }
        FIND_RESULT(const NAMES_ITEM &pattern2) : pattern(pattern2) { }        
void operator=(const FIND_RESULT &f)
        {
            __builtin_memcpy(this,&f,sizeof(FIND_RESULT));
        }
    };
    void find(const ATOM &pattern, FIND_RESULT &f) {
        FIND_RESULT result(pattern.getPredItem());
        f = result;
    }
};
class INTERPRET {
    ATOMSET positive;
public:
    void findInPositivePart(const ATOM &pattern, ATOMSET::FIND_RESULT &f)
    {
        positive.find(pattern,f);
    }
};
struct GINTERPRET {
    void isTrue (void);
};
extern INTERPRET J;
static GINTERPRET *I;
void printQueryI() {
    ATOM pattern;
    ATOMSET::FIND_RESULT f;
    J.findInPositivePart(pattern,f);
    I->isTrue();
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2006-11-29 19:47 -------
Also fails with 32bit.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 GCC target triplet|x86_64-*-*                  |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug c++/30017] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2006-11-29 19:50 -------
__builtin_memcpy(this,&f,sizeof(FIND_RESULT)); is most likely the cause.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[pinskia at gcc dot gnu dot org]

------- Comment #7 from pinskia at gcc dot gnu dot org  2006-11-30 04:18 -------
This is obvious what caused it:
+2006-11-28  Jan Hubicka  <jh@suse.cz>
+
+       * builtins.c: Include tree-flow.h.
+       (fold_builtin_memory_op): Be more aggressive on converting memcpy to
+       assignment; convert memmove to memcpy for sizes greater than 1 where
+       alignment of operands prohibit the partial overlap.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jh at suse dot cz
          Component|c++                         |middle-end
            Summary|ICE in cp_expr_size, at     |[4.3 Regression] ICE in
                   |cp/cp-objcp-common.c:101    |cp_expr_size, at cp/cp-
                   |                            |objcp-common.c:101
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[hubicka at gcc dot gnu dot org]

------- Comment #8 from hubicka at gcc dot gnu dot org  2006-11-30 13:06 -------
Probably my bug, I guess the same as shown in dlv nightly tester.
I am looking into it.


-- 

hubicka at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |hubicka at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-11-30 13:06:25
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2006-11-30 13:42 -------
Yes, it's a reduced testcase from the DVL failure.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[hubicka at ucw dot cz]

------- Comment #10 from hubicka at ucw dot cz  2006-11-30 14:04 -------
Subject: Re:  [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

Hi,
the problem is conversion of:
            __builtin_memcpy(this,&f,sizeof(FIND_RESULT));
to assignment.  It triggers sanity check in cp-objcp-common.c that
things of this nature are not initialized directly by backend.  I am not
quite sure if such a check is neccesarily needed and I think in such an
side cases it is correct to silently accept the code (it has undefined
behaviour anyway, right?) as proposed in the patch.

Alternatively we can come up with some alternate way. What however
worries me too is why this look is needed at first place.
How does langhook.expr_size differ from TYPE_SIZE_UNIT?  Perhaps the
memcpy folder needs to be expanded to use TYPE_SIZE_UNIT too?

Honza

        * cp-objcp-common.c (cp_expr_size): Do not sanity check that size of
certain
        classes does not matter.
Index: cp/cp-objcp-common.c
===================================================================
*** cp/cp-objcp-common.c        (revision 119360)
--- cp/cp-objcp-common.c        (working copy)
*************** cp_expr_size (tree exp)
*** 79,105 ****

    if (CLASS_TYPE_P (type))
      {
-       /* The backend should not be interested in the size of an expression
-        of a type with both of these set; all copies of such types must go
-        through a constructor or assignment op.  */
-       gcc_assert (!TYPE_HAS_COMPLEX_INIT_REF (type)
-                 || !TYPE_HAS_COMPLEX_ASSIGN_REF (type)
-                 /* But storing a CONSTRUCTOR isn't a copy.  */
-                 || TREE_CODE (exp) == CONSTRUCTOR
-                 /* And, the gimplifier will sometimes make a copy of
-                    an aggregate.  In particular, for a case like:
- 
-                       struct S { S(); };
-                       struct X { int a; S s; };
-                       X x = { 0 };
- 
-                    the gimplifier will create a temporary with
-                    static storage duration, perform static
-                    initialization of the temporary, and then copy
-                    the result.  Since the "s" subobject is never
-                    constructed, this is a valid transformation.  */
-                 || CP_AGGREGATE_TYPE_P (type));
- 
        /* This would be wrong for a type with virtual bases, but they are
         caught by the assert above.  */
        return (is_empty_class (type)
--- 79,84 ----


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[hubicka at ucw dot cz]

------- Comment #11 from hubicka at ucw dot cz  2006-11-30 14:06 -------
Subject: Re:  [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

> 
> 
> ------- Comment #10 from hubicka at ucw dot cz  2006-11-30 14:04 -------
> Subject: Re:  [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101
> 
> Hi,
> the problem is conversion of:
>             __builtin_memcpy(this,&f,sizeof(FIND_RESULT));
> to assignment.  It triggers sanity check in cp-objcp-common.c that
> things of this nature are not initialized directly by backend.  I am not
> quite sure if such a check is neccesarily needed and I think in such an
> side cases it is correct to silently accept the code (it has undefined
> behaviour anyway, right?) as proposed in the patch.
> 
> Alternatively we can come up with some alternate way. What however
yes, there is alternate way ;) WHat I intended to say here is that we
can strenghten the tests in builtins.c to disable the conversion, but
the tests tested in the hook are C++ specific, so I am not sure if there
is some interface.

Honza


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2006-11-30 14:08 -------
The code has not undefined behavior, but I think removing the check is ok (it's
certainly not supposed to trigger _inside_ the assignment operator or the
copy constructor).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[pinskia at gcc dot gnu dot org]

------- Comment #13 from pinskia at gcc dot gnu dot org  2006-11-30 18:31 -------
(In reply to comment #12)
> The code has not undefined behavior, but I think removing the check is ok (it's
> certainly not supposed to trigger _inside_ the assignment operator or the
> copy constructor).

Actually I don't think removing that check is ok.  Allows us to find problems
with the gimplifier, when it is trying to create a temprorary variable when it
should not.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[jh at suse dot cz]

------- Comment #14 from jh at suse dot cz  2006-11-30 18:54 -------
Subject: Re:  [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

> > The code has not undefined behavior, but I think removing the check is ok (it's
> > certainly not supposed to trigger _inside_ the assignment operator or the
> > copy constructor).
> 
> Actually I don't think removing that check is ok.  Allows us to find problems
> with the gimplifier, when it is trying to create a temprorary variable when it
> should not.

Well, actually we probably need to decide on one thing - ie if we want
to allow such a moves later at gimple form.  I would say that we want to
as we want to make gimple language indepdendent in longer form and thus
classes are just aggregates as of any kind and assigning them is safe.
If we don't want to, we need to come with check prohibiting builtins.c
to construct the assignment.
If we want to, we need to find way to either relax or elliminate the
check or move it to better place.  I still wonder why do we need
expr_size hook that late in compilation?
(ie using TYPE_UNIT_SIZE instead would solve the problem too)

Honza


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[pinskia at gcc dot gnu dot org]

------- Comment #15 from pinskia at gcc dot gnu dot org  2007-05-23 02:26 -------
I can no longer reproduce this issue on the mainline, at least with the reduced
testcase or with the orginal reduced testcase.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[mmitchel at gcc dot gnu dot org]

------- Comment #16 from mmitchel at gcc dot gnu dot org  2007-06-29 17:46 -------
If this can no longer be reproduced, let's close the bug.

(I believe the assertion in question is important.  If the gimplifier is ever
making copies of classes with assignment operators/copy constructors, we are
doing something wrong.  It is never safe to make such a copy.  Of course, the
front end needs to make that clear to the gimplifier by setting appropriate
bits on the type.)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017

[Bug middle-end/30017] [4.3 Regression] ICE in cp_expr_size, at cp/cp-objcp-common.c:101

[mmitchel at gcc dot gnu dot org]

------- Comment #17 from mmitchel at gcc dot gnu dot org  2007-08-10 00:23 -------
Closed, since Andrew says this can no longer be reproduced.


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |WORKSFORME


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30017