Message ID | 20180419182721.GA8577@tucnak |
---|---|
State | New |
Headers | show |
Series | Fix ICE in tinst_level refcounting introduced in r259457 (PR c++/85462) | expand |
Ok. On Thu, Apr 19, 2018, 12:27 PM Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > The following testcase ICEs starting with r259457 PR80290 fix. > > The problem is that the refcount is just 8-bit and if we need more than 256 > refcounts for one tinst_level, we fail an assertion. > > As discovered by Richard, the in_system_header_p member is write-only > since r138031: > grep in_system_header_p cp/*.[ch] > cp/cp-tree.h: bool in_system_header_p; > cp/pt.c: new_level->in_system_header_p = in_system_header_at > (input_location); > so we can easily even without using bitfields enlarge the refcount to > 16-bits and make it far less likely to hit the assertion. > > This patch implements also saturation, once the refcount goes to 65535, it > becomes immutable and we then never return the tinst_level into the free > list. It doesn't matter that much, because it is still GC allocated and > can > be freed during ggc_collect if nothing refers to it. > > I've tested the patch on the testcase also with 8-bit refcount and 255 as > rlimit_infinity. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-04-19 Jakub Jelinek <jakub@redhat.com> > > PR c++/85462 > * cp-tree.h (tinst_level): Remove in_system_header_p member, > change refcount member from unsigned char to unsigned short, > add refcount_infinity static data member, adjust comments. > * pt.c (tinst_level::refcount_infinity): Define. > (inc_refcount_use): Remove assert, don't increment if refcount > is already refcount_infinity, adjust comment. > (dec_refcount_use): Remove assert, don't decrement if refcount > is refcount_infinity, adjust comment. > (push_tinst_level_loc): Formatting fix. > > * g++.dg/cpp0x/pr85462.C: New test. > > --- gcc/cp/cp-tree.h.jj 2018-04-19 09:42:44.409864659 +0200 > +++ gcc/cp/cp-tree.h 2018-04-19 12:33:25.339085905 +0200 > @@ -5927,14 +5927,19 @@ struct GTY((chain_next ("%h.next"))) tin > /* The location where the template is instantiated. */ > location_t locus; > > - /* errorcount+sorrycount when we pushed this level. */ > + /* errorcount + sorrycount when we pushed this level. */ > unsigned short errors; > > - /* True if the location is in a system header. */ > - bool in_system_header_p; > + /* Count references to this object. If refcount reaches > + refcount_infinity value, we don't increment or decrement the > + refcount anymore, as the refcount isn't accurate anymore. > + The object can be still garbage collected if unreferenced from > + anywhere, which might keep referenced objects referenced longer than > + otherwise necessary. Hitting the infinity is rare though. */ > + unsigned short refcount; > > - /* Count references to this object. */ > - unsigned char refcount; > + /* Infinity value for the above refcount. */ > + static const unsigned short refcount_infinity = (unsigned short) ~0; > }; > > bool decl_spec_seq_has_spec_p (const cp_decl_specifier_seq *, > cp_decl_spec); > --- gcc/cp/pt.c.jj 2018-04-18 10:45:22.901720592 +0200 > +++ gcc/cp/pt.c 2018-04-19 12:33:17.704080557 +0200 > @@ -8945,15 +8945,14 @@ tinst_level::to_list () > return ret; > } > > -/* Increment OBJ's refcount. */ > +const unsigned short tinst_level::refcount_infinity; > + > +/* Increment OBJ's refcount unless it is already infinite. */ > static tinst_level * > inc_refcount_use (tinst_level *obj) > { > - if (obj) > - { > - ++obj->refcount; > - gcc_assert (obj->refcount != 0); > - } > + if (obj && obj->refcount != tinst_level::refcount_infinity) > + ++obj->refcount; > return obj; > } > > @@ -8966,15 +8965,16 @@ tinst_level::free (tinst_level *obj) > tinst_level_freelist ().free (obj); > } > > -/* Decrement OBJ's refcount. If it reaches zero, release OBJ's DECL > - and OBJ, and start over with the tinst_level object that used to be > - referenced by OBJ's NEXT. */ > +/* Decrement OBJ's refcount if not infinite. If it reaches zero, release > + OBJ's DECL and OBJ, and start over with the tinst_level object that > + used to be referenced by OBJ's NEXT. */ > static void > dec_refcount_use (tinst_level *obj) > { > - while (obj && !--obj->refcount) > + while (obj > + && obj->refcount != tinst_level::refcount_infinity > + && !--obj->refcount) > { > - gcc_assert (obj->refcount+1 != 0); > tinst_level *next = obj->next; > tinst_level::free (obj); > obj = next; > @@ -10143,8 +10143,7 @@ push_tinst_level_loc (tree tldcl, tree t > new_level->tldcl = tldcl; > new_level->targs = targs; > new_level->locus = loc; > - new_level->errors = errorcount+sorrycount; > - new_level->in_system_header_p = in_system_header_at (input_location); > + new_level->errors = errorcount + sorrycount; > new_level->next = NULL; > new_level->refcount = 0; > set_refcount_ptr (new_level->next, current_tinst_level); > --- gcc/testsuite/g++.dg/cpp0x/pr85462.C.jj 2018-04-19 > 19:16:50.994498502 +0200 > +++ gcc/testsuite/g++.dg/cpp0x/pr85462.C 2018-04-19 > 19:20:38.961594082 +0200 > @@ -0,0 +1,38 @@ > +// PR c++/85462 > +// { dg-do compile { target c++11 } } > + > +template <class T> struct D { using d = T *; }; > +template <class, class, class> struct E; > +template <class T, class U> struct E<T, U, U> { using d = typename > D<T>::d; }; > +template <class T> struct G { using d = typename E<T, int, int>::d; }; > +template <class T, class U> typename G<T>::d foo (U); > +#define A(n) class A##n {}; > +#define B(n) A(n##0) A(n##1) A(n##2) A(n##3) A(n##4) A(n##5) A(n##6) > A(n##7) A(n##8) A(n##9) > +#define C(n) B(n##0) B(n##1) B(n##2) B(n##3) B(n##4) B(n##5) B(n##6) > B(n##7) B(n##8) B(n##9) > +#define D(n) C(n##0) C(n##1) C(n##2) C(n##3) C(n##4) > +D(1) > +class H; > +template <typename> > +struct I > +{ > + bool bar (); > +#undef A > +#define A(n) void f##n (A##n *); > +D(1) > + void baz (); > +}; > +A1000 v; > +template <typename T> > +bool I<T>::bar () > +{ > +#undef A > +#define A(n) A##n k##n = *foo<A##n> (v); f##n (&k##n); > +D(1) > + foo<H> (v); > + baz (); > + return false; > +} > +struct J : I<int> > +{ > + void qux () { bar (); } > +}; > > Jakub >
--- gcc/cp/cp-tree.h.jj 2018-04-19 09:42:44.409864659 +0200 +++ gcc/cp/cp-tree.h 2018-04-19 12:33:25.339085905 +0200 @@ -5927,14 +5927,19 @@ struct GTY((chain_next ("%h.next"))) tin /* The location where the template is instantiated. */ location_t locus; - /* errorcount+sorrycount when we pushed this level. */ + /* errorcount + sorrycount when we pushed this level. */ unsigned short errors; - /* True if the location is in a system header. */ - bool in_system_header_p; + /* Count references to this object. If refcount reaches + refcount_infinity value, we don't increment or decrement the + refcount anymore, as the refcount isn't accurate anymore. + The object can be still garbage collected if unreferenced from + anywhere, which might keep referenced objects referenced longer than + otherwise necessary. Hitting the infinity is rare though. */ + unsigned short refcount; - /* Count references to this object. */ - unsigned char refcount; + /* Infinity value for the above refcount. */ + static const unsigned short refcount_infinity = (unsigned short) ~0; }; bool decl_spec_seq_has_spec_p (const cp_decl_specifier_seq *, cp_decl_spec); --- gcc/cp/pt.c.jj 2018-04-18 10:45:22.901720592 +0200 +++ gcc/cp/pt.c 2018-04-19 12:33:17.704080557 +0200 @@ -8945,15 +8945,14 @@ tinst_level::to_list () return ret; } -/* Increment OBJ's refcount. */ +const unsigned short tinst_level::refcount_infinity; + +/* Increment OBJ's refcount unless it is already infinite. */ static tinst_level * inc_refcount_use (tinst_level *obj) { - if (obj) - { - ++obj->refcount; - gcc_assert (obj->refcount != 0); - } + if (obj && obj->refcount != tinst_level::refcount_infinity) + ++obj->refcount; return obj; } @@ -8966,15 +8965,16 @@ tinst_level::free (tinst_level *obj) tinst_level_freelist ().free (obj); } -/* Decrement OBJ's refcount. If it reaches zero, release OBJ's DECL - and OBJ, and start over with the tinst_level object that used to be - referenced by OBJ's NEXT. */ +/* Decrement OBJ's refcount if not infinite. If it reaches zero, release + OBJ's DECL and OBJ, and start over with the tinst_level object that + used to be referenced by OBJ's NEXT. */ static void dec_refcount_use (tinst_level *obj) { - while (obj && !--obj->refcount) + while (obj + && obj->refcount != tinst_level::refcount_infinity + && !--obj->refcount) { - gcc_assert (obj->refcount+1 != 0); tinst_level *next = obj->next; tinst_level::free (obj); obj = next; @@ -10143,8 +10143,7 @@ push_tinst_level_loc (tree tldcl, tree t new_level->tldcl = tldcl; new_level->targs = targs; new_level->locus = loc; - new_level->errors = errorcount+sorrycount; - new_level->in_system_header_p = in_system_header_at (input_location); + new_level->errors = errorcount + sorrycount; new_level->next = NULL; new_level->refcount = 0; set_refcount_ptr (new_level->next, current_tinst_level); --- gcc/testsuite/g++.dg/cpp0x/pr85462.C.jj 2018-04-19 19:16:50.994498502 +0200 +++ gcc/testsuite/g++.dg/cpp0x/pr85462.C 2018-04-19 19:20:38.961594082 +0200 @@ -0,0 +1,38 @@ +// PR c++/85462 +// { dg-do compile { target c++11 } } + +template <class T> struct D { using d = T *; }; +template <class, class, class> struct E; +template <class T, class U> struct E<T, U, U> { using d = typename D<T>::d; }; +template <class T> struct G { using d = typename E<T, int, int>::d; }; +template <class T, class U> typename G<T>::d foo (U); +#define A(n) class A##n {}; +#define B(n) A(n##0) A(n##1) A(n##2) A(n##3) A(n##4) A(n##5) A(n##6) A(n##7) A(n##8) A(n##9) +#define C(n) B(n##0) B(n##1) B(n##2) B(n##3) B(n##4) B(n##5) B(n##6) B(n##7) B(n##8) B(n##9) +#define D(n) C(n##0) C(n##1) C(n##2) C(n##3) C(n##4) +D(1) +class H; +template <typename> +struct I +{ + bool bar (); +#undef A +#define A(n) void f##n (A##n *); +D(1) + void baz (); +}; +A1000 v; +template <typename T> +bool I<T>::bar () +{ +#undef A +#define A(n) A##n k##n = *foo<A##n> (v); f##n (&k##n); +D(1) + foo<H> (v); + baz (); + return false; +} +struct J : I<int> +{ + void qux () { bar (); } +};