Take the following C++ code, with -O1 we ICE right now (This is forwarded from PR 22277 because the orginal bug there was fixed): struct A { char c; int i; }; A a; struct B { char c, d; }; union C { A *p; B *q; C() : p(&a) {} char& foo() { return q->d; } }; void bar() { C().foo() = 0; }
Confirmed, because this is forwarding from one bug to another.
first_vi_for_offset Just to get a search off the comments to find this.
We inline all and get void bar() () { char & D.1777; struct B * D.1776; char & D.1773; union C D.1772; <bb 0>: D.1772.p = &a; D.1776_5 = D.1772.q; D.1777_6 = &D.1776_5->d; D.1773_7 = D.1777_6; *D.1773_7 = 0; return; } where we access the padding between c and i in struct A through an object of type B. This must be invalid code. Even if it is, the gcc_unreachable () is probably not a good idea - Danny, how should we deal with this? Ignore references to padding and just return NULL from first_vi_for_offset? There's a similar problem in get_constraint_for_component_ref which I hit with array-aliasing and work-around by *************** get_constraint_for_component_ref (tree t *** 2075,2082 **** } /* assert that we found *some* field there. The user couldn't be accessing *only* padding. */ ! ! gcc_assert (curr); } else if (dump_file && (dump_flags & TDF_DETAILS)) --- 2076,2084 ---- } /* assert that we found *some* field there. The user couldn't be accessing *only* padding. */ ! /* Still the user could access one past the end of an array ! embedded in a struct resulting in accessing *only* padding. */ ! /* gcc_assert (curr); */ } else if (dump_file && (dump_flags & TDF_DETAILS))
Created attachment 9471 [details] Fix This fixes the ICE. If this is the right approach I'll put this through bootstrap & regtesting.
With the patch we make _Z3barv: .LFB6: pushl %ebp .LCFI0: movl %esp, %ebp .LCFI1: movb $0, a+1 popl %ebp ret out of it btw, which looks ok.
Subject: Re: [4.1 Regression] ICE in first_vi_for_offset, at tree-ssa-structalias.c:2858 On Thu, 2005-08-11 at 10:44 +0000, rguenth at gcc dot gnu dot org wrote: > ------- Additional Comments From rguenth at gcc dot gnu dot org 2005-08-11 10:44 ------- > We inline all and get > > void bar() () > { > char & D.1777; > struct B * D.1776; > char & D.1773; > union C D.1772; > > <bb 0>: > D.1772.p = &a; > D.1776_5 = D.1772.q; > D.1777_6 = &D.1776_5->d; > D.1773_7 = D.1777_6; > *D.1773_7 = 0; > return; > > } > > where we access the padding between c and i in struct A through an > object of type B. This must be invalid code. Even if it is, the > gcc_unreachable () is probably not a good idea - Danny, how should > we deal with this? Ignore references to padding and just return > NULL from first_vi_for_offset? I know what to do here. I've just been waiting as long as possible to remove the assert because it also catches real bugs. I actually plan on removing it today, and have bootstrapped and tested a patch to do so. > > There's a similar problem in get_constraint_for_component_ref which > I hit with array-aliasing and work-around by > > *************** get_constraint_for_component_ref (tree t > *** 2075,2082 **** > } > /* assert that we found *some* field there. The user couldn't be > accessing *only* padding. */ > ! > ! gcc_assert (curr); > } > else > if (dump_file && (dump_flags & TDF_DETAILS)) > --- 2076,2084 ---- > } > /* assert that we found *some* field there. The user couldn't be > accessing *only* padding. */ > ! /* Still the user could access one past the end of an array > ! embedded in a struct resulting in accessing *only* padding. */ > ! /* gcc_assert (curr); */ You are right, but this can only occur with your patch, since it wouldn't have had fields before, and thus it would have overlapped with the actual variable, giving the right answer. :)
Subject: Bug 22615 CVSROOT: /cvs/gcc Module name: gcc Changes by: dberlin@gcc.gnu.org 2005-08-14 19:23:57 Modified files: gcc : ChangeLog tree-ssa-structalias.c Added files: gcc/testsuite/g++.dg/tree-ssa: pr22615.C Log message: 2005-08-14 Daniel Berlin <dberlin@dberlin.org> Fix PR tree-optimization/22615 * tree-ssa-structalias.c (solution_set_add): Handle first_vi_for_offset returning NULL. (do_da_constraint): Ditto. (do_sd_constraint): Ditto. (do_ds_constraint): Ditto (find_func_aliases): Ditto. (build_constraint_graph): RHS is allowed be ANYTHING. (first_vi_for_offset): Return NULL if we couldn't find anything at the offset. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9728&r2=2.9729 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-structalias.c.diff?cvsroot=gcc&r1=2.26&r2=2.27 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr22615.C.diff?cvsroot=gcc&r1=NONE&r2=1.1
Fixed