[Bug tree-optimization/21407] New: [4.1 Regression] wrong code with upcast in C++

[Bug tree-optimization/21407] New: [4.1 Regression] wrong code with upcast in C++

[pinskia at gcc dot gnu dot org]

The is huge bug which effects a large amount of C++, we should not abort.
extern "C" void abort(void);
struct T1 {int a, b; virtual void f(){}};
struct T : T1 { struct T1 w;  int b; };
void foo (struct T1 *p) { struct T *q = dynamic_cast<T*>(p); if (q->b != 2) abort (); }
int main () { struct T c; c.b = 2; foo (&c); return 0; }

-- 
           Summary: [4.1 Regression] wrong code with upcast in C++
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: critical
          Priority: P2
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pinskia at gcc dot gnu dot org
                CC: dberlin at gcc dot gnu dot org,gcc-bugs at gcc dot gnu
                    dot org


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.1.0
            Version|4.0.0                       |4.1.0


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-05-06 00:25 -------
Oh, I got the idea for this testcase after ssb asked about what the kernel was doing was valid and his 
example code but unlike that code, this is valid as C++ works slightly different than C.

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[belyshev at depni dot sinp dot msu dot ru]

------- Additional Comments From belyshev at depni dot sinp dot msu dot ru  2005-05-06 00:27 -------
Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-05-06 00:27:48
               date|                            |


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[dberlin at gcc dot gnu dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-05-09 13:58 -------
This is a call clobbering bug.
We don't pass the address of c to the call, we pass the address of some
substructure.
As a result, we don't think foo can touch c.b, when it can beause of the upcast.
Whee.

I'd imagine the following will fix it, though i haven't tested it yet

Index: tree-ssa-operands.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
retrieving revision 2.78
diff -u -p -r2.78 tree-ssa-operands.c
--- tree-ssa-operands.c 21 Apr 2005 18:05:26 -0000      2.78
+++ tree-ssa-operands.c 9 May 2005 13:57:44 -0000
@@ -1936,7 +1936,7 @@ note_addressable (tree var, stmt_ann_t s
      Otherwise, we take the address of all the subvariables, plus the real
      ones.  */

-  if (var && TREE_CODE (var) == COMPONENT_REF
+  if (0 && var && TREE_CODE (var) == COMPONENT_REF
       && (ref = okay_component_ref_for_subvars (var, &offset, &size)))
     {
       subvar_t sv;

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-05-10 13:16 -------
So what about this, in C:

-------------------------------
struct A
{
  int a;
};

struct B
{
  struct A sa;
  int b;
};

void foo1(void* ptr)
{
  ((struct A*)ptr)->a = 0;
}

void foo2(void* ptr)
{
  ((struct B*)ptr)->b = 0;
}

void bar(void)
{
  struct B sb;
  foo1(&sb);
  foo2(&sb.sa);
  foo1(&sb);
  foo2(&sb.sa);
}
-------------------------------

What is valid C and what is invalid C?


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu dot org


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-05-10 13:17 -------
Ehm, bar should obviously be:

-------------------------------
void bar(void)
{
  struct B sb;
  foo1(&sb);
  foo1(&sb.sa);
  foo2(&sb);
  foo2(&sb.sa);
}
-------------------------------

that is, I'm trying all the combinations :)

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-05-10 14:51 -------
(In reply to comment #4)
> void foo1(void* ptr)
> {
>   ((struct A*)ptr)->a = 0;
> }

Because you just violated C aliasing.  This is unlike C++ where upcasting is okay.

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[joseph at codesourcery dot com]

------- Additional Comments From joseph at codesourcery dot com  2005-05-10 14:59 -------
Subject: Re:  [4.1 Regression] wrong code with
 upcast in C++

On Tue, 10 May 2005, giovannibajo at libero dot it wrote:

> So what about this, in C:

Seems valid to me.  "A pointer to a structure object, suitably converted, 
points to its initial member (or if that member is a bit-field, then to 
the unit in which it resides), and vice versa." (C99 6.7.2.1#13).



-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[dberlin at gcc dot gnu dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-05-10 15:09 -------
And this would still work with the code i've written, because it's at offset 0
(which is why it's valid in C).

It would have worked before the fix, too.

It's only when you cast back *down* to the derived class (you'll note andrew's
example isn't really an upcast, because he's dynamic casting back down to a
derived class) that we were running into trouble.

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with upcast in C++

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-05-10 15:48 -------
In my example, B is a (sort-of) derived class and A is a (sort-of) base class. 
The C++ frontend should use a subobject at offset 0 to represent the base 
class. When you downcast through dynamic_cast, you are not doing anything 
different from what foo2() is doing, when called with &sb.sa (read: base class).

So what am I missing? What's the difference between:

----------------------------------
/* C code */
struct A { int a; };
struct B { struct A sa; int b; }

void foo(struct A* pa)
{
  ((struct B*)pa)->b = 0;
}

void bar(void)
{
  struct B sb;
  foo(&sb.sa);
}
----------------------------------

and

----------------------------------
/* C++ code */
struct A { int a; };
struct B : struct A { int b; }

void foo(struct A* pa)
{
  static_cast<B*>(pa)->b = 0;
}

void bar(void)
{
  struct B sb;
  foo(&sb);
}
----------------------------------

?

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2005-05-10 16:01 -------
In fact, the salias dump for the C++ testcase says:

---------------------------------------------------------------
;; Function void bar() (_Z3barv)

structure field tag SFT.2 created for var sb offset 0 size 32
void bar() ()
{
  struct B sb;

<bb 0>:
  foo (&sb.D.1580);
  return;

}
---------------------------------------------------------------

so it looks like the substructure *is* at offset zero.


-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dberlin at dberlin dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-05-10 16:07 -------
Subject: Re:  [4.1 Regression] wrong code with
	downcast in C++

On Tue, 2005-05-10 at 16:01 +0000, giovannibajo at libero dot it wrote:
> ------- Additional Comments From giovannibajo at libero dot it  2005-05-10 16:01 -------
> In fact, the salias dump for the C++ testcase says:
> 
> ---------------------------------------------------------------
> ;; Function void bar() (_Z3barv)
> 
> structure field tag SFT.2 created for var sb offset 0 size 32
> void bar() ()
> {
>   struct B sb;
> 
> <bb 0>:
>   foo (&sb.D.1580);
>   return;
> 
> }
> ---------------------------------------------------------------
> 
> so it looks like the substructure *is* at offset zero.
> 
> 

The only case we get anything wrong is when you can get back at the
*CONTAINING* structure's members legally (which in C++ means the derived
class) when they aren't in a place they will be clobbered anyway.


IE if you can take a pointer to somewhere in the middle of a structure,
subtract some bytes, and get back the containing structure, which is
effectively what dynamic cast is doing :)




-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-05-18 13:27 -------
Subject: Bug 21407

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	dberlin@gcc.gnu.org	2005-05-18 13:26:20

Modified files:
	gcc            : ChangeLog tree-ssa-operands.c 
Added files:
	gcc/testsuite/g++.dg/tree-ssa: pr21407.C 

Log message:
	2005-05-18  Daniel Berlin  <dberlin@dberlin.org>
	
	Fix PR tree-optimization/21407
	
	* tree-ssa-operands.c (note_addressable): Change
	COMPONENT_REF handling in response to aliasing
	discussion.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.8840&r2=2.8841
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-operands.c.diff?cvsroot=gcc&r1=2.82&r2=2.83
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr21407.C.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dberlin at gcc dot gnu dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-05-18 13:28 -------
Fixed

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dnovillo at gcc dot gnu dot org]

------- Additional Comments From dnovillo at gcc dot gnu dot org  2005-07-05 23:07 -------
(In reply to comment #13)
> Fixed
>
Have the URL for the aliasing discussion?


Thanks.

-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dberlin at gcc dot gnu dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-07-06 00:16 -------
It's in the ml archives, i'll try to find it.

Basically, Mark and friends believe that in C, it's legal to add or subtract
some bytes from a pointer to a field of a structure  and have a valid pointer to
some other field of that structure.

This idiom is apparently in common usage regardless.

Nathan queried the C++ committee, and they actually *don't* think it's legal in
C++, but we have no way of differentiating this from dynamic_cast at the moment,
so we have to be conservative.

Thus, a pointer to a field passed to a function must be considered to clobber
the entire structure that field is contained in (all the way up the structure
chain).
IE if a pointer to a structure field escapes, the entire structure instance escapes.




-- 


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

Re: [Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[Diego Novillo]

On Wed, Jul 06, 2005 at 12:16:20AM -0000, dberlin at gcc dot gnu dot org wrote:
> 
> ------- Additional Comments From dberlin at gcc dot gnu dot org  2005-07-06 00:16 -------
> It's in the ml archives, i'll try to find it.
> 
Thanks.  I remember the discussion, but I need some URL so that
we can reference it from the source code.

The comment in tree-ssa-operands.c:note_addressable references
this PR, but there's no link from the PR into the discussion.


Diego.

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dnovillo at redhat dot com]

------- Additional Comments From dnovillo at redhat dot com  2005-07-06 00:23 -------
Subject: Re:  [4.1 Regression] wrong code with downcast in C++

On Wed, Jul 06, 2005 at 12:16:20AM -0000, dberlin at gcc dot gnu dot org wrote:
> 
> ------- Additional Comments From dberlin at gcc dot gnu dot org  2005-07-06 00:16 -------
> It's in the ml archives, i'll try to find it.
> 
Thanks.  I remember the discussion, but I need some URL so that
we can reference it from the source code.

The comment in tree-ssa-operands.c:note_addressable references
this PR, but there's no link from the PR into the discussion.


Diego.


-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2005-07-06 00:42 -------
Subject: Re:  [4.1 Regression] wrong code with downcast in C++

"dberlin at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Nathan queried the C++ committee, and they actually *don't* think
| it's legal in C++, 

It is more accurate to say that Nathan queried the BSI panel (UK)
which is very different from the C++ committee (ISO/IEC WG21).
I, for one, have not seen any query posted to the C++ core reflector
nor any discussion on that topic.

-- Gaby


-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dberlin at dberlin dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-07-06 00:58 -------
Subject: Re:  [4.1 Regression] wrong code with
	downcast in C++


> | Nathan queried the C++ committee, and they actually *don't* think
> | it's legal in C++, 
> 
> It is more accurate to say that Nathan queried the BSI panel (UK)
> which is very different from the C++ committee (ISO/IEC WG21).
> I, for one, have not seen any query posted to the C++ core reflector
> nor any discussion on that topic.
> 

Well, whatever. It's not relevant anyway, since i'm sure people would
scream and yell if we disallowed it in C++, regardless of what any
panel/committee thinks :)




-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2005-07-06 01:16 -------
Subject: Re:  [4.1 Regression] wrong code with downcast in C++

"dberlin at dberlin dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Subject: Re:  [4.1 Regression] wrong code with
| 	downcast in C++
| 
| 
| > | Nathan queried the C++ committee, and they actually *don't* think
| > | it's legal in C++, 
| > 
| > It is more accurate to say that Nathan queried the BSI panel (UK)
| > which is very different from the C++ committee (ISO/IEC WG21).
| > I, for one, have not seen any query posted to the C++ core reflector
| > nor any discussion on that topic.
| > 
| 
| Well, whatever. It's not relevant anyway, since i'm sure people would
| scream and yell if we disallowed it in C++, regardless of what any
| panel/committee thinks :)

We are in violent agreement.

-- Gaby


-- 


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

[Bug tree-optimization/21407] [4.1 Regression] wrong code with downcast in C++

[dberlin at gcc dot gnu dot org]

------- Additional Comments From dberlin at gcc dot gnu dot org  2005-07-06 16:54 -------
http://gcc.gnu.org/ml/gcc-patches/2005-05/msg00796.html was the start of the
discussion

-- 


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