On Fri, Jun 20, 2014 at 09:01:14PM +0200, Jakub Jelinek wrote: > On Fri, Jun 20, 2014 at 07:36:41PM +0200, Marek Polacek wrote: > > 2014-06-20 Marek Polacek <pola...@redhat.com> > > > > * genpreds.c (verify_rtx_codes): New function. > > (main): Call it. > > * rtl.h (RTX_FLD_WIDTH, RTX_HWINT_WIDTH): Define. > > (struct rtx_def): Use them. > > Note, e.g. Coverity also complains about this stuff loudly too, > apparently not just in the problematic case where rtx_def is used > in a middle of structure, but also when used in flexible array > like spot. Most RTLs are allocated through rtx_alloc and the size > is determined from RTX_HDR_SIZE (i.e. offsetof) and/or RTX_CODE_SIZE, > so your rtl.h change IMHO shouldn't affect anything but make the > expmed.c init_expmed_rtl structure somewhat longer.
> > --- gcc/genpreds.c > > +++ gcc/genpreds.c > > @@ -1471,6 +1471,40 @@ parse_option (const char *opt) > > return 0; > > } > > > > +/* Verify RTX codes. We can't call fatal_error here, so call > > + gcc_unreachable after error to really abort. */ > > + > > +static void > > +verify_rtx_codes (void) > > +{ > > + unsigned int i, j; > > + > > + for (i = 0; i < NUM_RTX_CODE; i++) > > + if (strchr (GET_RTX_FORMAT (i), 'w') == NULL) > > + { > > + if (strlen (GET_RTX_FORMAT (i)) > RTX_FLD_WIDTH) > > + { > > + error ("insufficient size of RTX_FLD_WIDTH"); > > + gcc_unreachable (); > > I think it would be nice to be more verbose, i.e. say > which rtx has longer format string than RTX_FLD_WIDTH, and perhaps also > the size and RTX_FLD_WIDTH value. Also, can't you use internal_error > instead of error + gcc_unreachable ? > > So perhaps > internal_error ("%s format %s longer than RTX_FLD_WIDTH %d\n", > GET_RTX_NAME (i), GET_RTX_FORMAT (i), > (int) RTX_FLD_WIDTH); > ? Ok, that's much better. I actually can use internal_error - we have that function in both diagnostic.c and errors.c, while fatal_error is only in diagnostic.c. > > + } > > + } > > + else > > + { > > + const size_t len = strlen (GET_RTX_FORMAT (i)); > > + for (j = 0; j < len; j++) > > + if (GET_RTX_FORMAT (i)[j] != 'w') > > + { > > + error ("rtx format does not contain only hwint entries"); > > + gcc_unreachable (); > > + } > > + if (len > RTX_HWINT_WIDTH) > > + { > > + error ("insufficient size of RTL_MAX_HWINT_WIDTH"); > > + gcc_unreachable (); > > + } > > + } > > And similarly here. Fixed. > Otherwise, LGTM, but as I've been discussing this with Marek, > I'd prefer somebody else to review it. Sure. So can anybody look at this, please? 2014-06-20 Marek Polacek <pola...@redhat.com> * genpreds.c (verify_rtx_codes): New function. (main): Call it. * rtl.h (RTX_FLD_WIDTH, RTX_HWINT_WIDTH): Define. (struct rtx_def): Use them. diff --git gcc/genpreds.c gcc/genpreds.c index b14a4ac..7e62124 100644 --- gcc/genpreds.c +++ gcc/genpreds.c @@ -1471,6 +1471,36 @@ parse_option (const char *opt) return 0; } +/* Verify RTX codes. */ + +static void +verify_rtx_codes (void) +{ + unsigned int i, j; + + for (i = 0; i < NUM_RTX_CODE; i++) + if (strchr (GET_RTX_FORMAT (i), 'w') == NULL) + { + if (strlen (GET_RTX_FORMAT (i)) > RTX_FLD_WIDTH) + internal_error ("%s format %s longer than RTX_FLD_WIDTH %d\n", + GET_RTX_NAME (i), GET_RTX_FORMAT (i), + (int) RTX_FLD_WIDTH); + } + else + { + const size_t len = strlen (GET_RTX_FORMAT (i)); + for (j = 0; j < len; j++) + if (GET_RTX_FORMAT (i)[j] != 'w') + internal_error ("%s format %s should contain only w, but " + "has %c\n", GET_RTX_NAME (i), GET_RTX_FORMAT (i), + GET_RTX_FORMAT (i)[j]); + if (len > RTX_HWINT_WIDTH) + internal_error ("%s format %s longer than RTX_HWINT_WIDTH %d\n", + GET_RTX_NAME (i), GET_RTX_FORMAT (i), + (int) RTX_HWINT_WIDTH); + } +} + /* Master control. */ int main (int argc, char **argv) @@ -1518,5 +1548,7 @@ main (int argc, char **argv) if (have_error || ferror (stdout) || fflush (stdout) || fclose (stdout)) return FATAL_EXIT_CODE; + verify_rtx_codes (); + return SUCCESS_EXIT_CODE; } diff --git gcc/rtl.h gcc/rtl.h index 6ec91a8..3f2e774 100644 --- gcc/rtl.h +++ gcc/rtl.h @@ -264,6 +264,12 @@ struct GTY((variable_size)) hwivec_def { #define CWI_PUT_NUM_ELEM(RTX, NUM) \ (RTL_FLAG_CHECK1("CWI_PUT_NUM_ELEM", (RTX), CONST_WIDE_INT)->u2.num_elem = (NUM)) +/* The maximum number of entries in the FLD array in rtx. */ +#define RTX_FLD_WIDTH 8 + +/* The maximum number of entries in the HWINT array in rtx. */ +#define RTX_HWINT_WIDTH (MAX (REAL_WIDTH, 3)) + /* RTL expression ("rtx"). */ struct GTY((chain_next ("RTX_NEXT (&%h)"), @@ -378,8 +384,8 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"), The number of operands and their types are controlled by the `code' field, according to rtl.def. */ union u { - rtunion fld[1]; - HOST_WIDE_INT hwint[1]; + rtunion fld[RTX_FLD_WIDTH]; + HOST_WIDE_INT hwint[RTX_HWINT_WIDTH]; struct block_symbol block_sym; struct real_value rv; struct fixed_value fv; Marek