From patchwork Fri Jun 20 20:42:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 362345 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 6154E140086 for ; Sat, 21 Jun 2014 06:43:13 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=GrWe+69GHsMILzG2V pN9jnUPmCOAh/eJQKHKVdV8x4dMrLKr9CIIuQaR+lnvuOSeky0KL6g8QQvnYrjSc sy8CDhqoxSIsGeOuqd4zZWdnxvS4LLVDvHJW40PCksRXvx44t1tcM4meXVCtnTig C3BePsBBYRFJxcr2Q8kPZdFD4E= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=Urgx+AvNI87u2oK0WMQ0dkx DRgk=; b=eT0tg+2J5rrMXFYEkYMWsnHLP8Ftg61qAtftSJgC7Wtfh2XtmI87gLy nwbU56Y+xxLRXXBqEHcq/k9rR8Zw+OhPPswzEYEMKGo/yuwYizEm+7yFd8glub2K TvLy5X0xHvEhMxRZFVQBaSNo0ox/gD5BM9zcGMV5mKMnJ4h14YVk= Received: (qmail 12013 invoked by alias); 20 Jun 2014 20:43:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 12003 invoked by uid 89); 20 Jun 2014 20:43:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 20 Jun 2014 20:43:05 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5KKh37A016221 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 20 Jun 2014 16:43:04 -0400 Received: from redhat.com (ovpn-116-31.ams2.redhat.com [10.36.116.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5KKh0Ut014931 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Fri, 20 Jun 2014 16:43:02 -0400 Date: Fri, 20 Jun 2014 22:42:59 +0200 From: Marek Polacek To: Jakub Jelinek Cc: GCC Patches , Jeff Law , Richard Henderson Subject: Re: [PATCH] Fix arrays in rtx.u + add minor rtx verification Message-ID: <20140620204259.GE14420@redhat.com> References: <20140620173640.GC14420@redhat.com> <20140620190113.GD31640@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140620190113.GD31640@tucnak.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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 > > > > * 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 * genpreds.c (verify_rtx_codes): New function. (main): Call it. * rtl.h (RTX_FLD_WIDTH, RTX_HWINT_WIDTH): Define. (struct rtx_def): Use them. Marek 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;