This is GCC Bugzilla
This is GCC Bugzilla Version 2.20+
View Bug Activity | Format For Printing | Clone This Bug
When I compile ace542 with the actual snapshot of gcc41 I get an ICE when I use -O3. The last snapshot which works is gcc-4.1-20050604, the first that fails is gcc-4.1-20050611 Michael Cieslinski g++41f -O3 -c -o .obj/cubitS.o cubitS.ii cubitS.cpp: In static member function 'static void POA_Cubit::cube_struct_skel (TAO_ServerRequest&, void*, void*)': cubitS.cpp:4193: internal compiler error: in first_vi_for_offset, at tree-ssa- structalias.c:2585 Please submit a full bug report, with preprocessed source if appropriate. g++41f -v Using built-in specs. Target: x86_64-unknown-linux-gnu Configured with: ../gcc-4.1-20050709/configure --prefix=/usr/local/gcc41f --program-suffix=41f --with-arch=opteron --enable-languages=c,c++ --enable-checking Thread model: posix gcc version 4.1.0 20050709 (experimental)
Created an attachment (id=9272) [edit] preprocessed source
This might be fixed already but I have not tried to look into it yet.
Could be that this is a duplicate of bug22422; I will try the patch given there
I can't apply the patch or compile the actual source of tree-ssa-structalias.c, also I don't have cvs access so I can't check if this ICE still exists in the new software.
This case doesn't fail on my i686 x x86_64 cross for c++ as of today, so i'm going to guess it was the same bug. You should probably verify that the patch fixes it on your compiler, of course :) 06-11 is a bit old. Unfortunately, i'm pretty sure there have been some invasive changes elsewhere since then so you can't just copy the tree-ssa-structalias.c file from mainline over. (in particular, i think this snapshot is before the removal of var_ann->uid) Any snapshot after 2005-07-10 should be relatively easy to backport patches to if necessary now that we are in stage 3
I build ggc from the snapshots every week. Tocompile the program I used 20050709. I use the older compilers only to find when this ICE first occured.
Subject: Re: [4.1 Regression] ICE: in first_vi_for_offset, at tree-ssa-structalias.c:2585 with -O3 On Thu, 2005-07-14 at 14:57 +0000, micis at gmx dot de wrote: > ------- Additional Comments From micis at gmx dot de 2005-07-14 14:56 ------- > I build ggc from the snapshots every week. > Tocompile the program I used 20050709. > I use the older compilers only to find when this ICE first occured. > > Yeah, but the patches i'm thinking of that prevents backporting went in on 07-10 :(
After some tweaking I can reproduce the ICE on i686-pc-linux-gnu with todays version of mainline. So it's not a dupe of PR22422.
Subject: Re: [4.1 Regression] ICE: in first_vi_for_offset, at tree-ssa-structalias.c:2585 with -O3 On Thu, 2005-07-14 at 16:53 +0000, reichelt at gcc dot gnu dot org wrote: > ------- Additional Comments From reichelt at gcc dot gnu dot org 2005-07-14 16:53 ------- > After some tweaking I can reproduce the ICE on i686-pc-linux-gnu with > todays version of mainline. So it's not a dupe of PR22422. > > But the file posted does *not* reproduce it for me
Confirmed. Reduced testcase (compile with -O) ===================================== struct X { int i0, i1; char c; }; struct A { int i; char c0, c1; virtual ~A(); }; struct B : virtual A {}; struct C : B { X x; void bar(X y) { x = y; } }; void foo() { C().bar(X()); } ===================================== On i686-pc-linux-gnu this results in: PR22488.cc: In function 'void foo()': PR22488.cc:24: internal compiler error: in first_vi_for_offset, at tree-ssa-structalias.c:2858 Please submit a full bug report, [etc.]
Only reproducible with the reduced testcase on 32bit targets.
I believe something is actually wrong in the field layout for the minimized testcase. We have fields that overlap. Adding mark mitchell. Mark, if you use the attached aliashelp.diff, and -fdump-tree-alias-all, and look at alias1, you'll see: ;; Function void foo() (_Z3foov) Entering type NULL at offset 0 Adding field i0 at offset 0 Adding field i1 at offset 32 Adding field c at offset 64 Entering type NULL at offset 0 Entering type NULL at offset 0 Adding field _vptr.B at offset 0 Entering type NULL at offset 32 Adding field _vptr.A at offset 32 Adding field i at offset 64 Adding field c0 at offset 96 Adding field c1 at offset 104 Entering type NULL at offset 32 Adding field i0 at offset 32 Adding field i1 at offset 64 Adding field c at offset 96 Entering type NULL at offset 128 Adding field _vptr.A at offset 128 Adding field i at offset 160 Adding field c0 at offset 192 Adding field c1 at offset 200 Entering type NULL at offset 0 Adding field i0 at offset 0 Adding field i1 at offset 32 Adding field c at offset 64 (The NULL is because the type is unnamed). Note that _vptr.A overlaps with i0, etc. Is this right?
Created an attachment (id=9337) [edit] Patch to dump field walking
http://gcc.gnu.org/ml/gcc-patches/2005-07/msg01608.html does not help
This looks like a front-end bug rather than a tree opt bug.
With the actual snapshot I get a segfault: GNU C++ version 4.1.0 20050826 (experimental) (x86_64-unknown-linux-gnu) compiled by GNU C version 4.1.0 20050826 (experimental). GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 Compiler executable checksum: 48dcabcf3efa906449d3613e11227389 Program received signal SIGSEGV, Segmentation fault. do_structure_copy (lhsop=Variable "lhsop" is not available. ) at ../../gcc-4.1-20050826/gcc/tree-ssa-structalias.c:2320 2320 temprhs.var = q->id; (gdb) where #0 do_structure_copy (lhsop=Variable "lhsop" is not available. ) at ../../gcc-4.1-20050826/gcc/tree-ssa-structalias.c:2320 #1 0x000000000086dad2 in find_func_aliases (t=0x2a9a184460, ai=Variable "ai" is not available. ) at ../../gcc-4.1-20050826/gcc/tree-ssa-structalias.c:2808 #2 0x000000000086f582 in compute_points_to_sets (ai=0x12801a0) at ../../gcc- 4.1-20050826/gcc/tree-ssa-structalias.c:3620 #3 0x0000000000580aea in compute_may_aliases () at ../../gcc-4.1- 20050826/gcc/tree-ssa-alias.c:263 #4 0x0000000000852f3c in execute_one_pass (pass=0xc0cfe0) at ../../gcc-4.1- 20050826/gcc/passes.c:808 #5 0x000000000085306c in execute_pass_list (pass=0xc0cfe0) at ../../gcc-4.1- 20050826/gcc/passes.c:840 #6 0x000000000085307e in execute_pass_list (pass=0xc0caa0) at ../../gcc-4.1- 20050826/gcc/passes.c:841 #7 0x000000000054be65 in tree_rest_of_compilation (fndecl=0x2a963d5100) at ../../gcc-4.1-20050826/gcc/tree-optimize.c:419 #8 0x00000000004cab48 in expand_body (fn=0x2a963d5100) at ../../gcc-4.1- 20050826/gcc/cp/semantics.c:3008 #9 0x0000000000897e26 in cgraph_expand_function (node=0x2a9a65a9a0) at ../../gcc-4.1-20050826/gcc/cgraphunit.c:1037 #10 0x0000000000899f3b in cgraph_optimize () at ../../gcc-4.1- 20050826/gcc/cgraphunit.c:1103 #11 0x000000000047b86f in cp_finish_file () at ../../gcc-4.1- 20050826/gcc/cp/decl2.c:3102 #12 0x0000000000524e7a in c_common_parse_file (set_yydebug=Variable "set_yydebug" is not available. ) at ../../gcc-4.1-20050826/gcc/c-opts.c:1117 #13 0x0000000000826c4e in toplev_main (argc=Variable "argc" is not available. ) at ../../gcc-4.1-20050826/gcc/toplev.c:971 #14 0x0000003ef781bf6a in __libc_start_main () from /lib64/tls/libc.so.6
Try Index: tree-ssa-structalias.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/tree-ssa-structalias.c,v retrieving revision 2.27 diff -c -3 -p -r2.27 tree-ssa-structalias.c *** tree-ssa-structalias.c 14 Aug 2005 19:23:56 -0000 2.27 --- tree-ssa-structalias.c 29 Aug 2005 12:12:21 -0000 *************** do_simple_structure_copy (const struct c *** 2317,2322 **** --- 2317,2324 ---- q = get_varinfo (temprhs.var); fieldoffset = p->offset - pstart; q = first_vi_for_offset (q, q->offset + fieldoffset); + if (!q) + continue; temprhs.var = q->id; process_constraint (new_constraint (templhs, temprhs)); } to workaround this.
Subject: Re: [4.1 Regression] ICE: in first_vi_for_offset, at tree-ssa-structalias.c:2585 with -O3 On Mon, 2005-08-29 at 12:13 +0000, rguenth at gcc dot gnu dot org wrote: > ------- Additional Comments From rguenth at gcc dot gnu dot org 2005-08-29 12:13 ------- > Try > > Index: tree-ssa-structalias.c > =================================================================== > RCS file: /cvs/gcc/gcc/gcc/tree-ssa-structalias.c,v > retrieving revision 2.27 > diff -c -3 -p -r2.27 tree-ssa-structalias.c > *** tree-ssa-structalias.c 14 Aug 2005 19:23:56 -0000 2.27 > --- tree-ssa-structalias.c 29 Aug 2005 12:12:21 -0000 > *************** do_simple_structure_copy (const struct c > *** 2317,2322 **** > --- 2317,2324 ---- > q = get_varinfo (temprhs.var); > fieldoffset = p->offset - pstart; > q = first_vi_for_offset (q, q->offset + fieldoffset); > + if (!q) > + continue; > temprhs.var = q->id; > process_constraint (new_constraint (templhs, temprhs)); > } > > to workaround this. > This will generate bad code. This is one of the places where first_vi_for_offset should *never* fail. (it's not okay for it to fail during constraint generation. it's okay to fail during solving). As i noted in the bug, the real problem here seems to be the FE is generating overlaps when it shouldn't be (or it's using magic that we aren't to calculate the offsets, and hten is lying about it to the middle end :P)
I've changed the summary to match what the actual bug appears to be, at least until a C++ person tells us whether it's supposed to be generating these overlaps or not.
*** Bug 23729 has been marked as a duplicate of this bug. ***
And here is a testcase for 64bits: struct S02 { bool V01, V02; }; struct S03 {unsigned char V03; int V04; }; struct S05 { static void F01 ( int &P01, void *P02, void *P03 ); }; struct S07 {}; struct S09 : public virtual S02 {}; struct S10 : public S09 { S10(int &P04, S07* P05, S05* P06, S03 &P07); void F02(S03 F02); S03 V05; }; void S10::F02(S03 F02) { V05 = F02; } void S05::F01(int &P04, void* P02, void* P03) { S05 *P09 = (S05*)P02; S03 V06; S03 P07; S07 *P08 = (S07*)P03; S10 F03(P04, P08, P09, P07); S03 V07 = V06; F03.F02(V07); }
*** Bug 23949 has been marked as a duplicate of this bug. ***
I don't see what the alleged front-end bug is in this PR. The layout I would expect for C on IA32 GNU/Linux would be: 0: B's vptr 4: i0 8: i1 12: c 16: A's vptr 20: i 24: c0 25: c1 That is exactly what I seem to see when looking at the output of the class layout algorithm. In the dump in comment #12 I see no indication of overlap between "_vptr.A" (which is located at offset 128, as predicated above) and "i0" (which is, strangely, listed twice at offset zero, which is not correct). I see no evidence in looking at the TYPE_FIELDS list that i0 is at offset zero. Please explain exactly what you think the front end defect is here, in terms of front end data structures. What FIELD_DECL has an incorrect offset?
We have a var of type C named D.2117 <var_decl 0x4022cec8 D.2117 type <record_type 0x40212ac8 C addressable tree_2 needs-constructing type_1 type_4 type_5 BLK size <integer_cst 0x40210ed0 constant invariant 224> unit size <integer_cst 0x40210f00 constant invariant 28> align 32 symtab 0 alias set 21 fields <field_decl 0x40212ebc D.1814 type <record_type 0x40212170 B> ignored decl_6 BLK file /home/dberlin/22488.cc line 18 size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 offset_align 128 offset <integer_cst 0x40169198 constant invariant 0> bit offset <integer_cst 0x40169960 constant invariant 0> context <record_type 0x40212ac8 C> chain <field_decl 0x40212b24 x>> needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=1 use_template=0 interface-unknown pointer_to_this <pointer_type 0x40212c38> chain <type_decl 0x401eedd0 C>> addressable ignored BLK file /home/dberlin/22488.cc line 26 size <integer_cst 0x40210ed0 224> unit size <integer_cst 0x40210f00 28> align 32 context <function_decl 0x40214680 foo>> The first field of this is a field D.2184 of type B <field_decl 0x40212ebc D.1814 type <record_type 0x40212170 B addressable tree_2 needs-constructing type_1 type_4 type_5 BLK size <integer_cst 0x40169660 constant invariant 128> unit size <integer_cst 0x40169678 constant invariant 16> align 32 symtab 0 alias set 2 fields <field_decl 0x40212450 _vptr.B type <pointer_type 0x401f005c> unsigned virtual SI file /home/dberlin/22488.cc line 15 size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 offset_align 128 offset <integer_cst 0x40169198 constant invariant 0> bit offset <integer_cst 0x40169960 constant invariant 0> context <record_type 0x40212170 B> chain <type_decl 0x401eec98 B>> needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=1 use_template=0 interface-unknown pointer_to_this <pointer_type 0x40212228> chain <type_decl 0x401eec30 B>> ignored decl_6 BLK file /home/dberlin/22488.cc line 18 size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 offset_align 128 offset <integer_cst 0x40169198 0> bit offset <integer_cst 0x40169960 0> context <record_type 0x40212ac8 C> chain <field_decl 0x40212b24 x>> This field of type B is at offset (offset = 0 + bit_offset = 0) from our var of type C The first field of B is _vptr.B: <field_decl 0x40212450 _vptr.B type <pointer_type 0x401f005c type <pointer_type 0x401e5f74 __vtbl_ptr_type type <function_type 0x401e5f18> unsigned SI size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 symtab 0 alias set 7 pointer_to_this <pointer_type 0x401f005c>> unsigned SI size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 symtab 0 alias set 4> unsigned virtual SI file /home/dberlin/22488.cc line 15 size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 offset_align 128 offset <integer_cst 0x40169198 type <integer_type 0x4017b000 unsigned int> constant invariant 0> bit offset <integer_cst 0x40169960 type <integer_type 0x4017b05c bit_size_type> constant invariant 0> context <record_type 0x40212170 B> chain <type_decl 0x401eec98 B>> It is at offset 0 from our field of type B, which is at offset 0 from our var of type C. The next field after B is a type_decl. The next field after the type_decl is field_decl 0x40212564 D.1781 <field_decl 0x40212564 D.1781 type <record_type 0x4020e9b4 A addressable tree_2 needs-constructing type_1 type_4 type_5 BLK size <integer_cst 0x40169a80 constant invariant 96> unit size <integer_cst 0x40169a98 constant invariant 12> align 32 symtab 0 alias set 3 fields <field_decl 0x4020ee04 _vptr.A type <pointer_type 0x401f005c> unsigned virtual SI file /home/dberlin/22488.cc line 8 size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 offset_align 128 offset <integer_cst 0x40169198 constant invariant 0> bit offset <integer_cst 0x40169960 constant invariant 0> context <record_type 0x4020e9b4 A> chain <field_decl 0x4020ea10 i>> needs-constructor needs-destructor X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-only pointer_to_this <pointer_type 0x4020ebdc> chain <type_decl 0x401eea90 A>> ignored decl_6 BLK file /home/dberlin/22488.cc line 15 size <integer_cst 0x40210630 type <integer_type 0x4017b05c bit_size_type> constant invariant 80> unit size <integer_cst 0x40210678 type <integer_type 0x4017b000 unsigned int> constant invariant 10> align 32 offset_align 128 offset <integer_cst 0x40169198 0> bit offset <integer_cst 0x401693f0 32> context <record_type 0x40212170 B>> It is at offset 32 from our field of type B, which is at offset 0 from our var of type C. So total offset = 32 for this field. It is of type A. The first field of type A is _vptr.A. <field_decl 0x4020ee04 _vptr.A type <pointer_type 0x401f005c type <pointer_type 0x401e5f74 __vtbl_ptr_type type <function_type 0x401e5f18> unsigned SI size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 symtab 0 alias set 7 pointer_to_this <pointer_type 0x401f005c>> unsigned SI size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 symtab 0 alias set 4> unsigned virtual SI file /home/dberlin/22488.cc line 8 size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 offset_align 128 offset <integer_cst 0x40169198 type <integer_type 0x4017b000 unsigned int> constant invariant 0> bit offset <integer_cst 0x40169960 type <integer_type 0x4017b05c bit_size_type> constant invariant 0> context <record_type 0x4020e9b4 A> chain <field_decl 0x4020ea10 i>> This field is at offset 0 from our field of type A, which is at offset 32 from our field of type B, which is at offset 0 from our var of type C. So total offset = 32 from var C for this field. Let's skip the rest of the fields of our field of type A since they don't mtater for showing the overlap. Pop back up to the field of type B, then pop back up to our original var of type C. The next *field* of type C is <field_decl 0x40212b24 x type <record_type 0x4020e844 X type_1 type_5 BLK size <integer_cst 0x40169a80 constant invariant 96> unit size <integer_cst 0x40169a98 constant invariant 12> align 32 symtab 0 alias set 22 fields <field_decl 0x4020e8a0 i0 type <integer_type 0x4017b284 int> nonlocal decl_3 SI file /home/dberlin/22488.cc line 3 size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 offset_align 128 offset <integer_cst 0x40169198 constant invariant 0> bit offset <integer_cst 0x40169960 constant invariant 0> context <record_type 0x4020e844 X> chain <field_decl 0x4020e8fc i1>> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown pointer_to_this <pointer_type 0x402164ac> reference_to_this <reference_type 0x4021633c> chain <type_decl 0x401ee958 X>> used nonlocal decl_3 BLK file /home/dberlin/22488.cc line 19 size <integer_cst 0x40169a80 96> unit size <integer_cst 0x40169a98 12> align 32 offset_align 128 offset <integer_cst 0x40169198 0> bit offset <integer_cst 0x401693f0 32> context <record_type 0x40212ac8 C> chain <type_decl 0x401eee38 C>> This says that we have a field decl named x of type X at offset 32 from our var of type C. It's first field is i0: <field_decl 0x4020e8a0 i0 type <integer_type 0x4017b284 int public SI size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 symtab 0 alias set 5 precision 32 min <integer_cst 0x401693a8 -2147483648> max <integer_cst 0x401693c0 2147483647> pointer_to_this <pointer_type 0x4017bc38>> nonlocal decl_3 SI file /home/dberlin/22488.cc line 3 size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 offset_align 128 offset <integer_cst 0x40169198 type <integer_type 0x4017b000 unsigned int> constant invariant 0> bit offset <integer_cst 0x40169960 type <integer_type 0x4017b05c bit_size_type> constant invariant 0> context <record_type 0x4020e844 X> chain <field_decl 0x4020e8fc i1>> This field is at offset 0 from our var of field of type X which is at offset 32 from our var of type C. Thus, it overlaps with _vptr.A. Does your frontend output look like this? Are we expected to be adding the deepest offset from the first depth first search to the following ones when we pop back up or something? If so, the docs should be clarified so they state "structure type" instead of "structure", since i consider the *structure* here to be the variable (as would, i argue, anyone who programs instead of works on type systems :P), in which case, the offsets are wrong. /* In a FIELD_DECL, this is the field position, counting in bytes, of the byte containing the bit closest to the beginning of the structure. */ #define DECL_FIELD_OFFSET(NODE) (FIELD_DECL_CHECK (NODE)->field_decl.offset) /* In a FIELD_DECL, this is the offset, in bits, of the first bit of the field from DECL_FIELD_OFFSET. */ #define DECL_FIELD_BIT_OFFSET(NODE) (FIELD_DECL_CHECK (NODE)->field_decl.bit_offset)
Subject: Re: [4.1 Regression] C++ generates incorrect overlapping fields dberlin at gcc dot gnu dot org wrote: > ------- Additional Comments From dberlin at gcc dot gnu dot org 2005-09-27 20:42 ------- > We have a var of type C named D.2117 > The first field of this is a field D.2184 of type B > This field of type B is at offset (offset = 0 + bit_offset = 0) from our var of > type C Yes. > The first field of B is _vptr.B: > > It is at offset 0 from our field of type B, which is at offset 0 from our var of > type C. Yes; _vptr.B is indeed at offset zero relative to the enclosing C. > The next field after the type_decl is field_decl 0x40212564 D.1781 Not for me; the next field is: <field_decl 0x2a95a5e600 x type <record_type 0x2a95a52210 X type_1 type_5 BLK When I eventually get to the field of type "A" it says: <field_decl 0x2a95a2be40 D.1817 type <record_type 0x2a95a22b00 A addressable tree_2 needs-constructing type_1 type_4 type_5 BLK ... offset <integer_cst 0x2a958e4f60 type <integer_type 0x2a958f6000 unsigned int> constant invariant 16> bit offset <integer_cst 0x2a95901540 0> context <record_type 0x2a95a31580 C>> Now, I'm looking at the type after layout_class_type. It's possible that at some later point it gets stomped. Can you take a look to see what you see after layout_class_type?
I meant the next field of *B* was a type_decl fields <field_decl 0x40212450 _vptr.B type <pointer_type 0x401f005c type <pointer_type 0x401e5f74 __vtbl_ptr_type> unsigned SI size <integer_cst 0x401693f0 constant invariant 32> unit size <integer_cst 0x40169180 constant invariant 4> align 32 symtab 0 alias set -1> unsigned virtual SI file /home/dberlin/22488.cc line 15 size <integer_cst 0x401693f0 32> unit size <integer_cst 0x40169180 4> align 32 offset_align 128 offset <integer_cst 0x40169198 constant invariant 0> bit offset <integer_cst 0x40169960 constant invariant 0> context <record_type 0x40212170 B> chain <type_decl 0x401eec98 B type <record_type 0x40212170 B> nonlocal decl_4 VOID file /home/dberlin/22488.cc line 15 align 1 context <record_type 0x40212170 B> chain <field_decl 0x40212564 D.1781>>> (vptr.b, chained to type_decl,chained to D.1781) In C, we have b -> x -> type_decl for C However, the questions still remains, what is the offset supposed to be relative to? The direct context, or the outermost containing type? The way the docs are worded, seems to say the outermost containing (IE the structure that contains them all). Otherwise, to calculate the offset of the second field you have to walk the *entire* nest of types/fields that make up the first field of a structure, which isn't what we are doing. The docs just say "relative to the structure", which is unhelpful :)
Subject: Re: [4.1 Regression] C++ generates incorrect overlapping fields dberlin at gcc dot gnu dot org wrote: > However, the questions still remains, what is the offset supposed to be relative to? The beginning of the type in which the field is contained, which I think means, in your parlance: > The direct context, or the outermost containing type? the "direct context". In other words, if C has a field of type B at offset 32, and B has an field "f" with offset 16, then the offset of B::f relative to C is 48. (That's the only thing that makes any sense; otherwise, we'd have to make a copy of B every time it appeared as field in some other type.)
Subject: Re: [4.1 Regression] C++ generates incorrect overlapping fields On Wed, 2005-09-28 at 02:06 +0000, mark at codesourcery dot com wrote: > ------- Additional Comments From mark at codesourcery dot com 2005-09-28 02:06 ------- > Subject: Re: [4.1 Regression] C++ generates incorrect overlapping > fields > > dberlin at gcc dot gnu dot org wrote: > > > However, the questions still remains, what is the offset supposed to be relative to? > > The beginning of the type in which the field is contained, which I think > means, in your parlance: The parlance is that of GCC. I didn't invent DECL_CONTEXT :) > > > The direct context, or the outermost containing type? > > the "direct context". Okay, then this is my bug, not yours :) I'll change the summary in a moment. > > In other words, if C has a field of type B at offset 32, and B has an > field "f" with offset 16, then the offset of B::f relative to C is 48. > (That's the only thing that makes any sense; otherwise, we'd have to > make a copy of B every time it appeared as field in some other type.) Well, yes, if you link everything together using a list like that, and only store the offset in a shared field decl. But if you had a nice vector of std::pair<offset, field>, you wouldn't have to copy everything. :) > > >
Assigning to Daniel Berlin by his request.
> If so, are those fields of "A in B" really *there*, at those offsets > from the beginning of type B? Sort of. Because B would otherwise need a virtual table pointer, it just uses the one in A. So, there is a vptr at offset zero from the start of B. However, there's no actual A object at the same address as B because A is a virtual base. There's certainly no A::i at offset four from the start of B, for example. > The layout you had posted in the bug matches the offsets when we process > the type A field of C, but not the type A field of type B. Ugh. We create two versions of class types: the "complete" class type and the "as-base" type. (Look for CLASSTYPE_AS_BASE.) The as-base type does not include virtual bases, and the B field in C should have the B-as-base type, not the complete B type. The B-as-base type does not have the A parts. In fact, if you look at layout_class_type, and, in particular, build_base_field, you'll see it does use the as-base type. However, at the end of layout_class_type, we have: /* Now that we're done with layout, give the base fields the real types. */ and that installs the complete type. Jason added this in 2004: + 2004-04-30 Jason Merrill <jason@redhat.com> + + Refer to base members using COMPONENT_REFs where possible. + * class.c (build_simple_base_path): New fn. + (build_base_path): Use it for non-virtual base references. + (layout_class_type): Change base fields to their real type + after layout is done. I think this is a hack so that later, when we use the COMPONENT_REFs, we don't have to cast from the as-base type back to the nominal C++ type. (Of course, this would also be cleaner with a real C++-lowering pass.) However, I think the right thing is to have the cast; as you've discovered, Jason's approach is lying about what's really there. Jason?
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly mmitchel at gcc dot gnu dot org wrote: > I think this is a hack so that later, when we use the COMPONENT_REFs, we don't > have to cast from the as-base type back to the nominal C++ type. (Of course, > this would also be cleaner with a real C++-lowering pass.) > > However, I think the right thing is to have the cast; as you've discovered, > Jason's approach is lying about what's really there. Jason? My intent was to use COMPONENT_REFs where it's close enough to the truth, and casts where it isn't. The middle/back end produced significantly better code for COMPONENT_REFs than casts when I made the change, particularly with aliasing; I'm not sure if it's gotten any better since then. And besides, my approach isn't lying at all about this testcase; since there's only one appearance of the virtual base, it appears exactly where the field lists say it will. Daniel agreed that the problem is in his code. Jason
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly On Wed, 2005-09-28 at 19:39 +0000, jason at redhat dot com wrote: > ------- Additional Comments From jason at redhat dot com 2005-09-28 19:39 ------- > Subject: Re: [4.1 Regression] push_fields_onto_fieldstack > calculates offset incorrectly > > mmitchel at gcc dot gnu dot org wrote: > > I think this is a hack so that later, when we use the COMPONENT_REFs, we don't > > have to cast from the as-base type back to the nominal C++ type. (Of course, > > this would also be cleaner with a real C++-lowering pass.) > > > > However, I think the right thing is to have the cast; as you've discovered, > > Jason's approach is lying about what's really there. Jason? > > My intent was to use COMPONENT_REFs where it's close enough to the > truth, and casts where it isn't. The middle/back end produced > significantly better code for COMPONENT_REFs than casts when I made the > change, particularly with aliasing; I'm not sure if it's gotten any > better since then. > > And besides, my approach isn't lying at all about this testcase; since > there's only one appearance of the virtual base, it appears exactly > where the field lists say it will. Daniel agreed that the problem is in > his code. > No, it actually isn't. I sent mark a followup mail, that didn't get put into the bug report for some reason. The type looks like this: Main type of var: C First field of C: field of type B ( <field_decl 0x40212ebc D.1814 type <record_type 0x40212170 B ... chain <field_decl 0x40212b24 x>) First field of B : _vptr.B ( <field_decl 0x40212450 _vptr.B ... chain <type_decl 0x401eec98>) Second field of B: type_decl ( <type_decl 0x401eec98 B ... chain <field_decl 0x40212564 D.1781>) Third field of B: field of type A ( <field_decl 0x40212564 D.1781 ... chain NULL)> Second field of C: field of type X (<field_decl 0x40212b24 x type <record_type 0x4020e844 X ... chain <type_decl 0x401eee38 C>) Third field of C: type_decl (<type_decl 0x401eee38 C ... chain <field_decl 0x4021605c D.1817>) Fourth field of C: field of type A (<field_decl 0x4021605c D.1817 type <record_type 0x4020e9b4 A .. chain NULL) This causes us to walk type A twice (once as the third field of B, once as the fourth field of C), and where we get the offsets wrong, since we process the fields of type A twice. I don't see why type A appears in type C except through type B Is this because of virtual inheritance? If so, are those fields of "A in B" really *there*, at those offsets from the beginning of type B? The layout you had posted in the bug matches the offsets when we process the type A field of C, but not the type A field of type B. IE type A appears in two places. It's the first place that screws us up.
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly jason at redhat dot com wrote: > And besides, my approach isn't lying at all about this testcase; since > there's only one appearance of the virtual base, it appears exactly > where the field lists say it will. Daniel agreed that the problem is in > his code. What I meant by "lying" (an admittedly overly contentious choice of word) was that the field for B-in-C is marked as having the complete B type due to the code at the end of layout_class_type. However, the A virtual base isn't located in that B; it's located after the data members of C. So, the base field for B is incorrect. I agree that using COMPONENT_REFs is good, but I think that the FIELD_DECL for B should have the as-base type. Then, when you actually form the COMPONENT_REF, you should do the equivalent of: *(complete B*)&c.b_base That will give the expression the correct type from the front-end's point of view, but not result in inaccuracy with respect to the TYPE_FIELDS for C. Obviously, we need to make sure that B-as-base complete-B are in the same alias set. (It might be even better just to have the front end consider B-as-base and complete-B as the same type, so that the expression could have the more-accurate B-as-base type. But, that would be a huge change to the front end, so I think we have no choice but to use the complete-B type for an expression like "*(B*)&c".)
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly On Wed, 2005-09-28 at 20:29 +0000, mark at codesourcery dot com wrote: > ------- Additional Comments From mark at codesourcery dot com 2005-09-28 20:28 ------- > Subject: Re: [4.1 Regression] push_fields_onto_fieldstack > calculates offset incorrectly > > jason at redhat dot com wrote: > > > And besides, my approach isn't lying at all about this testcase; since > > there's only one appearance of the virtual base, it appears exactly > > where the field lists say it will. Daniel agreed that the problem is in > > his code. > > What I meant by "lying" (an admittedly overly contentious choice of > word) was that the field for B-in-C is marked as having the complete B > type due to the code at the end of layout_class_type. However, the A > virtual base isn't located in that B; it's located after the data > members of C. So, the base field for B is incorrect. If you give me guys a way to detect this case from the middle end, i'm happy to say "Don't walk this structure, it's not really there". :) > > I agree that using COMPONENT_REFs is good, but I think that the > FIELD_DECL for B should have the as-base type. Then, when you actually > form the COMPONENT_REF, you should do the equivalent of: > > *(complete B*)&c.b_base > > That will give the expression the correct type from the front-end's > point of view, but not result in inaccuracy with respect to the > TYPE_FIELDS for C. Obviously, we need to make sure that B-as-base > complete-B are in the same alias set. > > (It might be even better just to have the front end consider B-as-base > and complete-B as the same type, so that the expression could have the > more-accurate B-as-base type. But, that would be a huge change to the > front end, so I think we have no choice but to use the complete-B type > for an expression like "*(B*)&c".) > > >
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly dberlin at dberlin dot org wrote: >>>And besides, my approach isn't lying at all about this testcase; since >>>there's only one appearance of the virtual base, it appears exactly >>>where the field lists say it will. Daniel agreed that the problem is in >>>his code. >> >>What I meant by "lying" (an admittedly overly contentious choice of >>word) was that the field for B-in-C is marked as having the complete B >>type due to the code at the end of layout_class_type. However, the A >>virtual base isn't located in that B; it's located after the data >>members of C. So, the base field for B is incorrect. > > > If you give me guys a way to detect this case from the middle end, i'm > happy to say "Don't walk this structure, it's not really there". > > :) I don't think there's anyway to do that reliably.
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly mark at codesourcery dot com wrote: > > What I meant by "lying" (an admittedly overly contentious choice of > word) was that the field for B-in-C is marked as having the complete B > type due to the code at the end of layout_class_type. However, the A > virtual base isn't located in that B; it's located after the data > members of C. So, the base field for B is incorrect. Nah, "lying" is fair. If you lie to the compiler, it will have its revenge. > I agree that using COMPONENT_REFs is good, but I think that the > FIELD_DECL for B should have the as-base type. Then, when you actually > form the COMPONENT_REF, you should do the equivalent of: > > *(complete B*)&c.b_base > > That will give the expression the correct type from the front-end's > point of view, but not result in inaccuracy with respect to the > TYPE_FIELDS for C. Obviously, we need to make sure that B-as-base > complete-B are in the same alias set. The problem is that when you do that kind of type punning the optimizers get lost. > (It might be even better just to have the front end consider B-as-base > and complete-B as the same type, so that the expression could have the > more-accurate B-as-base type. But, that would be a huge change to the > front end, so I think we have no choice but to use the complete-B type > for an expression like "*(B*)&c".) I think it might actually work better to use the base type for most expressions, and only use the complete type when we know the complete type of the expression. Within the compiler, we ought to be able to express the complete type as a RECORD_TYPE containing fields of the as-base type and any vbases, so that the relationship between them is clear and we shouldn't need to do any ugly casting. Of course, this would also be a significant amount of work. Jason
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly jason at redhat dot com wrote: >>I agree that using COMPONENT_REFs is good, but I think that the >>FIELD_DECL for B should have the as-base type. Then, when you actually >>form the COMPONENT_REF, you should do the equivalent of: >> >> *(complete B*)&c.b_base >> >>That will give the expression the correct type from the front-end's >>point of view, but not result in inaccuracy with respect to the >>TYPE_FIELDS for C. Obviously, we need to make sure that B-as-base >>complete-B are in the same alias set. > > The problem is that when you do that kind of type punning the optimizers > get lost. We may have to find some way to give them a compass. :-) > I think it might actually work better to use the base type for most > expressions, and only use the complete type when we know the complete > type of the expression. Yes, I agree. The logically best thing would be that (possibly after some kind of lowering pass), the C++ front-end would generate expressions using the complete or as-base types, appropriately, accurately reflecting what we knew about what was there. > Within the compiler, we ought to be able to express the complete type as > a RECORD_TYPE containing fields of the as-base type and any vbases, so > that the relationship between them is clear and we shouldn't need to do > any ugly casting. Of course, this would also be a significant amount of > work. We almost do create the types as you suggest; we just don't use them for expressions. The as-base type is a copy of the complete type, before the vbases have been added, so it looks almost like a prefix of the complete type. The "almost" is because your code changes the types of the base fields in the complete type but not in the as-base type. So, I guess maybe we agree on the right plan. In particular: 1. When we see "T*" or "T&", use the as-base version of T for the type. Therefore, for expressions like "*p" where "p" has type "T*", we would give the expression the as-base version of T. However, for expressions like "t" (where "t" is of type "T"), use the complete version of "t". 2. Undo your change at the end of layout_class_type, so that base fields have the as-base type. We could implement (1) either by (a) doing that directly during parsing, etc., or (b) by handling it during genericization/gimplification. I vote for the latter. It should be relatively simple; do a pre-order walk of the expression tree, and adjust the types of things from the bottom up. I'm not sure what else we would need to do. We need to tell the optimizers that the as-base type can alias the complete type; I guess we just give them the same alias set. We also need the optimizers to understand that "*p = *q" is going to copy TYPE_SIZE (complete type) bytes...
As a 4.1 kludge, i can make the points-to analyzer do what it does for unions, which is to glob everything to a single variable for those classes where it has found two fields it thinks overlap. This will lose alias precision, but it won't be worse than what 4.0 was.
*** Bug 24190 has been marked as a duplicate of this bug. ***
(In reply to comment #38) > As a 4.1 kludge, i can make the points-to analyzer do what it does for unions, > which is to glob everything to a single variable for those classes where it has > found two fields it thinks overlap. > > This will lose alias precision, but it won't be worse than what 4.0 was. Yes please. This bug really annoys me ;)
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly rguenth at gcc dot gnu dot org wrote: > ------- Comment #40 from rguenth at gcc dot gnu dot org 2005-10-04 15:12 ------- > (In reply to comment #38) > >>As a 4.1 kludge, i can make the points-to analyzer do what it does for unions, >>which is to glob everything to a single variable for those classes where it has >>found two fields it thinks overlap. >> >>This will lose alias precision, but it won't be worse than what 4.0 was. > > > Yes please. This bug really annoys me ;) I agree that this is a reasonable solution. In fact, if you see conflicting field types for some chunk of memory, I don't think it would necessarily be unreasonable to just say that the whole object has some unknown-type alias information. C++ should provide an accurate representation of multiple inheritance to the middle-end; if it doesn't, it gets what it deserves, but it would be nice if programs still worked.
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly mark at codesourcery dot com wrote: > So, I guess maybe we agree on the right plan. In particular: > > 1. When we see "T*" or "T&", use the as-base version of T for the type. > Therefore, for expressions like "*p" where "p" has type "T*", we would > give the expression the as-base version of T. However, for expressions > like "t" (where "t" is of type "T"), use the complete version of "t". Yep. > 2. Undo your change at the end of layout_class_type, so that base fields > have the as-base type. Agreed. > We could implement (1) either by (a) doing that directly during parsing, > etc., or (b) by handling it during genericization/gimplification. I > vote for the latter. It should be relatively simple; do a pre-order > walk of the expression tree, and adjust the types of things from the > bottom up. > > I'm not sure what else we would need to do. We need to tell the > optimizers that the as-base type can alias the complete type; I guess we > just give them the same alias set. I was suggesting that instead of making the complete type and base type essentially copies of one another, that we make the complete type a wrapper around the base type. Proper alias semantics would fall out of that. > We also need the optimizers to > understand that "*p = *q" is going to copy TYPE_SIZE (complete type) > bytes... We don't do bitwise assignment if a class has a vptr, so whenever we generate something like that the complete type and base type should be the same. Jason
Subject: Re: [4.1 Regression] push_fields_onto_fieldstack calculates offset incorrectly jason at redhat dot com wrote: > I was suggesting that instead of making the complete type and base type > essentially copies of one another, that we make the complete type a > wrapper around the base type. Proper alias semantics would fall out of > that. That's a nice idea. It would make life a little more complicated for the front end; when seeing "a.b", you have to generate a double COMPONENT_REF, because the internal representation is a.as_base.b. But, that's a relatively localized change. We could actually do that at gimplification/genericization-time too, I guess, which would be a nice start towards proper lowering.
Subject: Bug 22488 CVSROOT: /cvs/gcc Module name: gcc Changes by: dberlin@gcc.gnu.org 2005-10-06 19:38:00 Modified files: gcc : ChangeLog tree-ssa-structalias.c Added files: gcc/testsuite/g++.dg/tree-ssa: pr22488.C Log message: 2005-10-06 Daniel Berlin <dberlin@dberlin.org> Fix PR tree-optimization/22488 * tree-ssa-structalias.c (check_for_overlaps): New function. (create_variable_info_for): Use it. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.10106&r2=2.10107 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-structalias.c.diff?cvsroot=gcc&r1=2.30&r2=2.31 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr22488.C.diff?cvsroot=gcc&r1=NONE&r2=1.1
Pushing the real fix off to 4.2 and unassigning, since it is worked around for 4.1
The real fix is a front-end fix.
Accepting so I remember to implement my proposal sometime soon.
Will not be fixed in 4.2.0; retargeting at 4.2.1.
Change target milestone to 4.2.3, as 4.2.2 has been released.
4.2.3 is being released now, changing milestones of open bugs to 4.2.4.
4.2.4 is being released, changing milestones to 4.2.5.
retargeting for 4.4.
Closing 4.1 branch.