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; }
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.
Confirmed.
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;
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?
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 :)
(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.
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).
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.
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); } ---------------------------------- ?
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.
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 :)
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
Fixed
(In reply to comment #13) > Fixed > Have the URL for the aliasing discussion? Thanks.
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.
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.
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
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 :)
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/ml/gcc-patches/2005-05/msg00796.html was the start of the discussion