On Tue, Aug 5, 2014 at 1:17 PM, Trevor Saunders <tsaund...@mozilla.com> wrote: > On Tue, Aug 05, 2014 at 10:45:38AM +0200, Richard Biener wrote: >> On Mon, Aug 4, 2014 at 3:04 PM, <tsaund...@mozilla.com> wrote: >> > From: Trevor Saunders <tsaund...@mozilla.com> >> > >> > Hi, >> > >> > It used to be that edge_var_maps held pointers to embedded vectors, but >> > now it holds vectors. This means that now instead of copying the >> > address of the embedded vector from the table we keep a pointer into the >> > table. However that's incorrect because we may expand the table when >> > inserting new into the map in which case our pointer into the map points >> > at freed memory. >> > >> > gcc/ >> > >> > * tree-ssa.c (redirect_edge_var_map_dup): copy the value in the >> > map for old before inserting new. >> > >> > testing ongoing on x86_64-unknown-linux-gnu, ok? >> >> Umm... can you explore what it takes to instead store some kind of >> indexes? > > into what?
No idea - I didn't look into the affected code not do I remember it off-head. >> That is, how is the above not an issue for other 'head's that may be >> live somewhere? > > what do you mean by other head's? Presumably nobody did something like > > void **p = pointer_map_contains (t, x); > void **q = pointer_map_insert (t, y); > *q = *p; > > because that would be broken, and that's the same problem we have here. > So assuming I didn't break anything else nobody else uses a pointer into > the table that they have after inserting elements. Ah, now I understand. I thought somebody might hold onto 'p' to some other random element. At least if that was valid "before". If not the patch is ok. Thanks, Richard. >> Or can you restore what "used to be"? > > I thought I had to bail out early if the map didn't contain old, but > when I actually look at the ddiff it looks like we delt with this before > by just inserting new and then getting old so yeah I can just make > things work more like they used to. Though maybe at some point we want > to try and have less elements in the table with empty vectors. > > Trev > >> >> Thanks, >> Richard. >> >> > Trev >> > >> > --- >> > gcc/tree-ssa.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c >> > index 920cbea..b949d48 100644 >> > --- a/gcc/tree-ssa.c >> > +++ b/gcc/tree-ssa.c >> > @@ -109,7 +109,11 @@ redirect_edge_var_map_dup (edge newe, edge olde) >> > if (!head) >> > return; >> > >> > - edge_var_maps->get_or_insert (newe).safe_splice (*head); >> > + /* Save what head points at because if we need to insert new into the >> > map we >> > + may end up expanding the table in which case head will no longer >> > point at >> > + valid memory. */ >> > + vec<edge_var_map> h = *head; >> > + edge_var_maps->get_or_insert (newe).safe_splice (h); >> > } >> > >> > >> > -- >> > 2.0.1 >> >
