From patchwork Sun Jul 18 07:36:29 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Tighten gengtype array error Date: Sat, 17 Jul 2010 21:36:29 -0000 From: Richard Sandiford X-Patchwork-Id: 59161 Message-Id: <87pqyls0le.fsf@firetop.home> To: gcc-patches@gcc.gnu.org In: http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00272.html I added an error to gengtype to diagnose array roots that could not be expressed as a single count and stride. Unfortunately, the error was to eager: it triggered for arrays that didn't actually need to be garbage-collected: http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01011.html This patch delays the error until we actually try to write such a root out. As penance, I've also fixed string roots so that they can be in arrays too (something that wasn't true before or after my original patch, and which currently causes silent wrong code). This is actually a necessary part of the patch, because the TYPE_STRING needs to cope with the new meaning of "v". Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested by adding string roots and too-complex roots to x_rtl and checking the gengtype behaved as expected. OK to install? Richard gcc/ * gengtype.c (start_root_entry): New function, split out from write_root. Check whether V is null and raise an error if so. (write_field_root): Check for V being null. Don't raise an error here; set V to null instead. (write_root): Update comment above function. Use start_root_entry. Index: gcc/gengtype.c =================================================================== --- gcc/gengtype.c 2010-07-17 17:49:15.000000000 +0100 +++ gcc/gengtype.c 2010-07-17 18:04:53.000000000 +0100 @@ -3174,6 +3174,39 @@ finish_root_table (struct flist *flp, co } } +/* Write the first three fields (pointer, count and stride) for + root NAME to F. V and LINE are as for write_root. + + Return true if the entry could be written; return false on error. */ + +static bool +start_root_entry (outf_p f, pair_p v, const char *name, struct fileloc *line) +{ + type_p ap; + + if (!v) + { + error_at_line (line, "`%s' is too complex to be a root", name); + return false; + } + + oprintf (f, " {\n"); + oprintf (f, " &%s,\n", name); + oprintf (f, " 1"); + + for (ap = v->type; ap->kind == TYPE_ARRAY; ap = ap->u.a.p) + if (ap->u.a.len[0]) + oprintf (f, " * (%s)", ap->u.a.len); + else if (ap == v->type) + oprintf (f, " * ARRAY_SIZE (%s)", v->name); + oprintf (f, ",\n"); + oprintf (f, " sizeof (%s", v->name); + for (ap = v->type; ap->kind == TYPE_ARRAY; ap = ap->u.a.p) + oprintf (f, "[0]"); + oprintf (f, "),\n"); + return true; +} + /* A subroutine of write_root for writing the roots for field FIELD_NAME, which has type FIELD_TYPE. Parameters F to EMIT_PCH are the parameters of the caller. */ @@ -3187,7 +3220,7 @@ write_field_root (outf_p f, pair_p v, ty subcomponent of V, we can mark any subarrays with a single stride. We're effectively treating the field as a global variable in its own right. */ - if (type == v->type) + if (v && type == v->type) { struct pair newv; @@ -3199,14 +3232,23 @@ write_field_root (outf_p f, pair_p v, ty /* Otherwise, any arrays nested in the structure are too complex to handle. */ else if (field_type->kind == TYPE_ARRAY) - error_at_line (line, "nested array `%s.%s' is too complex to be a root", - name, field_name); + v = NULL; write_root (f, v, field_type, ACONCAT ((name, ".", field_name, NULL)), has_length, line, if_marked, emit_pch); } /* Write out to F the table entry and any marker routines needed to - mark NAME as TYPE. The original variable is V, at LINE. + mark NAME as TYPE. V can be one of three values: + + - null, if NAME is too complex to represent using a single + count and stride. In this case, it is an error for NAME to + contain any gc-ed data. + + - the outermost array that contains NAME, if NAME is part of an array. + + - the C variable that contains NAME, if NAME is not part of an array. + + LINE is the line of the C source that declares the root variable. HAS_LENGTH is nonzero iff V was a variable-length array. IF_MARKED is nonzero iff we are building the root table for hash table caches. */ @@ -3291,22 +3333,10 @@ write_root (outf_p f, pair_p v, type_p t case TYPE_POINTER: { - type_p ap, tp; + type_p tp; - oprintf (f, " {\n"); - oprintf (f, " &%s,\n", name); - oprintf (f, " 1"); - - for (ap = v->type; ap->kind == TYPE_ARRAY; ap = ap->u.a.p) - if (ap->u.a.len[0]) - oprintf (f, " * (%s)", ap->u.a.len); - else if (ap == v->type) - oprintf (f, " * ARRAY_SIZE (%s)", v->name); - oprintf (f, ",\n"); - oprintf (f, " sizeof (%s", v->name); - for (ap = v->type; ap->kind == TYPE_ARRAY; ap = ap->u.a.p) - oprintf (f, "[0]"); - oprintf (f, "),\n"); + if (!start_root_entry (f, v, name, line)) + return; tp = type->u.p; @@ -3353,10 +3383,9 @@ write_root (outf_p f, pair_p v, type_p t case TYPE_STRING: { - oprintf (f, " {\n"); - oprintf (f, " &%s,\n", name); - oprintf (f, " 1, \n"); - oprintf (f, " sizeof (%s),\n", v->name); + if (!start_root_entry (f, v, name, line)) + return; + oprintf (f, " (gt_pointer_walker) >_ggc_m_S,\n"); oprintf (f, " (gt_pointer_walker) >_pch_n_S\n"); oprintf (f, " },\n");