On 06/20/14 13:01, 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,
Yes it does. IIRC, these warnings from Coverity were the cause of GCC reaching the #1 or #2 position across Red Hat's packages in terms of Coverity warnings. Not a good position to be in.


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.
Right. This comment was actually very helpful in that I wasn't aware of precisely which cases Marek was trying to address.

Presumably the [1] sizing is what prevents any compile-time checking of this?

No strong opinion on the internal_error vs error+unreachable.


--- 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 ?
Agreed, definitely indicate which RTX code is problematical.

Jeff

Reply via email to