From patchwork Wed May 11 05:39:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Monakov X-Patchwork-Id: 620896 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 3r4Q1h2ZMbz9s4x for ; Wed, 11 May 2016 15:40:07 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=L7UG/KfU; dkim-atps=neutral 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:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=XwkcY7UQ8tbo5u15 OB4NGelNcqUTwvPY/EtWOWo/7FymMpL+sRLB4zOmoxqGKg6wFWpRmeeyNsMh40RI hsq5bWTvKpB5CLxKaOcPb2me66BjoiX5fBGHlNv8OeUprCTUy5EI77byo24IqG/x oDgLrJWvnWUzbLM6X/GaAV3duiM= 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:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=U30pdwhXiJepecokRhHb3f eIMCA=; b=L7UG/KfUfRGTGeBkyAAGvkcLW8M6BvVH2Y8TOxMPgS1/qaEBdYFgWL YNmOEZJc/ofkEjCu5HVNZ3kqJ6JZzz47lq8f3oT53FjSWO7Be1KzBy92i0jX5NNJ hXnWyybbIvZ4qzE/RuDlVm18zRwgGUoo6/OT5E0qxNNAfXzYcq5mE= Received: (qmail 22827 invoked by alias); 11 May 2016 05:39:58 -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 22782 invoked by uid 89); 11 May 2016 05:39:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, T_FILL_THIS_FORM_SHORT autolearn=ham version=3.3.2 spammy=followups, H*Ad:U*aldyh, prototypes, Hernandez X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp.ispras.ru Received: from smtp.ispras.ru (HELO smtp.ispras.ru) (83.149.199.79) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 May 2016 05:39:52 +0000 Received: from [10.10.3.121] (unknown [83.149.199.91]) by smtp.ispras.ru (Postfix) with ESMTP id 63FA0203FA; Wed, 11 May 2016 08:39:49 +0300 (MSK) Date: Wed, 11 May 2016 08:39:49 +0300 (MSK) From: Alexander Monakov To: Bernd Schmidt cc: Aldy Hernandez , GCC Mailing List , "Vladimir N. Makarov" , gcc-patches , Andrey Belevantsev Subject: [PATCH] clean up insn-automata.c (was: Re: out of bounds access in insn-automata.c) In-Reply-To: <56FC2466.1060406@redhat.com> Message-ID: References: <56F23888.5080506@redhat.com> <56F2B57E.6080300@t-online.de> <56F3BEA5.1090007@redhat.com> <56F3ED30.1050407@redhat.com> <56F4B3DE.8000708@redhat.com> <56FC2466.1060406@redhat.com> User-Agent: Alpine 2.20 (LNX 67 2015-01-07) MIME-Version: 1.0 On Wed, 30 Mar 2016, Bernd Schmidt wrote: > On 03/25/2016 04:43 AM, Aldy Hernandez wrote: > > If Bernd is fine with this, I'm happy to retract my patch and any > > possible followups. I'm just interested in having no path causing a > > possible out of bounds access. If your patch will do that, I'm cool. > > I'll need to see that patch first to comment :-) Here's the proposed patch. I've found that there's only one user of the current fancy logic in output_internal_insn_code_evaluation: handling of NULL_RTX and const0_rtx is only useful for 'state_transition' (generated by output_trans_func), so it's possible to inline the extended handling there, and handle only plain non-null rtx_insns in output_internal_insn_code_evaluation. This change allows to remove extra checks and casting in output_internal_insn_latency_func, as done by the patch. As a nice bonus, it constrains prototypes of three automata functions to accept insn_rtx rather than just rtx. Bootstrapped and regtested on x86_64, OK? Thanks. Alexander * genattr.c (main): Change 'rtx' to 'rtx_insn *' in prototypes of 'insn_latency', 'maximal_insn_latency', 'min_insn_conflict_delay'. * genautomata.c (output_internal_insn_code_evaluation): Simplify. Move handling of non-insn arguments inline into the sole user: (output_trans_func): ...here. (output_min_insn_conflict_delay_func): Change 'rtx' to 'rtx_insn *' in emitted function prototype. (output_internal_insn_latency_func): Ditto. Simplify. (output_internal_maximal_insn_latency_func): Ditto. Delete always-unused argument. (output_insn_latency_func): Ditto. (output_maximal_insn_latency_func): Ditto. diff --git a/gcc/genattr.c b/gcc/genattr.c index 656a9a7..77380e7 100644 --- a/gcc/genattr.c +++ b/gcc/genattr.c @@ -240,11 +240,11 @@ main (int argc, const char **argv) printf ("/* Insn latency time on data consumed by the 2nd insn.\n"); printf (" Use the function if bypass_p returns nonzero for\n"); printf (" the 1st insn. */\n"); - printf ("extern int insn_latency (rtx, rtx);\n\n"); + printf ("extern int insn_latency (rtx_insn *, rtx_insn *);\n\n"); printf ("/* Maximal insn latency time possible of all bypasses for this insn.\n"); printf (" Use the function if bypass_p returns nonzero for\n"); printf (" the 1st insn. */\n"); - printf ("extern int maximal_insn_latency (rtx);\n\n"); + printf ("extern int maximal_insn_latency (rtx_insn *);\n\n"); printf ("\n#if AUTOMATON_ALTS\n"); printf ("/* The following function returns number of alternative\n"); printf (" reservations of given insn. It may be used for better\n"); @@ -290,8 +290,8 @@ main (int argc, const char **argv) printf (" state_transition should return negative value for\n"); printf (" the insn and the state). Data dependencies between\n"); printf (" the insns are ignored by the function. */\n"); - printf - ("extern int min_insn_conflict_delay (state_t, rtx, rtx);\n"); + printf ("extern int " + "min_insn_conflict_delay (state_t, rtx_insn *, rtx_insn *);\n"); printf ("/* The following function outputs reservations for given\n"); printf (" insn as they are described in the corresponding\n"); printf (" define_insn_reservation. */\n"); diff --git a/gcc/genautomata.c b/gcc/genautomata.c index dcde604..92c8b5c 100644 --- a/gcc/genautomata.c +++ b/gcc/genautomata.c @@ -8113,14 +8113,10 @@ output_internal_trans_func (void) /* Output code - if (insn != 0) - { - insn_code = dfa_insn_code (insn); - if (insn_code > DFA__ADVANCE_CYCLE) - return code; - } - else - insn_code = DFA__ADVANCE_CYCLE; + gcc_checking_assert (insn != 0); + insn_code = dfa_insn_code (insn); + if (insn_code >= DFA__ADVANCE_CYCLE) + return code; where insn denotes INSN_NAME, insn_code denotes INSN_CODE_NAME, and code denotes CODE. */ @@ -8129,21 +8125,12 @@ output_internal_insn_code_evaluation (const char *insn_name, const char *insn_code_name, int code) { - fprintf (output_file, "\n if (%s == 0)\n", insn_name); - fprintf (output_file, " %s = %s;\n\n", - insn_code_name, ADVANCE_CYCLE_VALUE_NAME); - if (collapse_flag) - { - fprintf (output_file, "\n else if (%s == const0_rtx)\n", insn_name); - fprintf (output_file, " %s = %s;\n\n", - insn_code_name, COLLAPSE_NDFA_VALUE_NAME); - } - fprintf (output_file, "\n else\n {\n"); - fprintf (output_file, - " %s = %s (as_a (%s));\n", - insn_code_name, DFA_INSN_CODE_FUNC_NAME, insn_name); - fprintf (output_file, " if (%s > %s)\n return %d;\n }\n", - insn_code_name, ADVANCE_CYCLE_VALUE_NAME, code); + fprintf (output_file, " gcc_checking_assert (%s != 0);\n" + " %s = %s (%s);\n" + " if (%s >= %s)\n return %d;\n", + insn_name, + insn_code_name, DFA_INSN_CODE_FUNC_NAME, insn_name, + insn_code_name, ADVANCE_CYCLE_VALUE_NAME, code); } @@ -8204,8 +8191,22 @@ output_trans_func (void) TRANSITION_FUNC_NAME, STATE_TYPE_NAME, STATE_NAME, INSN_PARAMETER_NAME); fprintf (output_file, "{\n int %s;\n", INTERNAL_INSN_CODE_NAME); - output_internal_insn_code_evaluation (INSN_PARAMETER_NAME, - INTERNAL_INSN_CODE_NAME, -1); + fprintf (output_file, "\n if (%s == 0)\n", INSN_PARAMETER_NAME); + fprintf (output_file, " %s = %s;\n", + INTERNAL_INSN_CODE_NAME, ADVANCE_CYCLE_VALUE_NAME); + if (collapse_flag) + { + fprintf (output_file, " else if (%s == const0_rtx)\n", + INSN_PARAMETER_NAME); + fprintf (output_file, " %s = %s;\n", + INTERNAL_INSN_CODE_NAME, COLLAPSE_NDFA_VALUE_NAME); + } + fprintf (output_file, " else\n {\n"); + fprintf (output_file, " %s = %s (as_a (%s));\n", + INTERNAL_INSN_CODE_NAME, DFA_INSN_CODE_FUNC_NAME, + INSN_PARAMETER_NAME); + fprintf (output_file, " if (%s > %s)\n return -1;\n }\n", + INTERNAL_INSN_CODE_NAME, ADVANCE_CYCLE_VALUE_NAME); fprintf (output_file, " return %s (%s, (struct %s *) %s);\n}\n\n", INTERNAL_TRANSITION_FUNC_NAME, INTERNAL_INSN_CODE_NAME, CHIP_NAME, STATE_NAME); } @@ -8297,7 +8298,7 @@ static void output_min_insn_conflict_delay_func (void) { fprintf (output_file, - "int\n%s (%s %s, rtx %s, rtx %s)\n", + "int\n%s (%s %s, rtx_insn *%s, rtx_insn *%s)\n", MIN_INSN_CONFLICT_DELAY_FUNC_NAME, STATE_TYPE_NAME, STATE_NAME, INSN_PARAMETER_NAME, INSN2_PARAMETER_NAME); fprintf (output_file, "{\n struct %s %s;\n int %s, %s, transition;\n", @@ -8366,10 +8367,12 @@ output_internal_insn_latency_func (void) decl_t decl; struct bypass_decl *bypass; - fprintf (output_file, "static int\n%s (int %s ATTRIBUTE_UNUSED,\n\tint %s ATTRIBUTE_UNUSED,\n\trtx %s ATTRIBUTE_UNUSED,\n\trtx %s ATTRIBUTE_UNUSED)\n", - INTERNAL_INSN_LATENCY_FUNC_NAME, INTERNAL_INSN_CODE_NAME, - INTERNAL_INSN2_CODE_NAME, "insn_or_const0", - "insn2_or_const0"); + fprintf (output_file, "static int\n" + "%s (int %s ATTRIBUTE_UNUSED, int %s ATTRIBUTE_UNUSED,\n" + "\trtx_insn *%s ATTRIBUTE_UNUSED, rtx_insn *%s ATTRIBUTE_UNUSED)\n", + INTERNAL_INSN_LATENCY_FUNC_NAME, + INTERNAL_INSN_CODE_NAME, INTERNAL_INSN2_CODE_NAME, + INSN_PARAMETER_NAME, INSN2_PARAMETER_NAME); fprintf (output_file, "{\n"); if (DECL_INSN_RESERV (advance_cycle_insn_decl)->insn_num == 0) @@ -8378,32 +8381,6 @@ output_internal_insn_latency_func (void) return; } - fprintf (output_file, " if (%s >= %s || %s >= %s)\n return 0;\n", - INTERNAL_INSN_CODE_NAME, ADVANCE_CYCLE_VALUE_NAME, - INTERNAL_INSN2_CODE_NAME, ADVANCE_CYCLE_VALUE_NAME); - - /* We've now rejected the case that - INTERNAL_INSN_CODE_NAME >= ADVANCE_CYCLE_VALUE_NAME - i.e. that - insn_code >= DFA__ADVANCE_CYCLE, - and similarly for insn2_code. */ - fprintf (output_file, - " /* Within output_internal_insn_code_evaluation, the generated\n" - " code sets \"code\" to NDFA__COLLAPSE for const0_rtx, and\n" - " NDFA__COLLAPSE > DFA__ADVANCE_CYCLE. Hence we can't be\n" - " dealing with const0_rtx instances at this point. */\n"); - if (collapse_flag) - fprintf (output_file, - " gcc_assert (NDFA__COLLAPSE > DFA__ADVANCE_CYCLE);\n"); - fprintf (output_file, - (" gcc_assert (insn_or_const0 != const0_rtx);\n" - " rtx_insn *%s ATTRIBUTE_UNUSED = safe_as_a (insn_or_const0);\n"), - INSN_PARAMETER_NAME); - fprintf (output_file, - (" gcc_assert (insn2_or_const0 != const0_rtx);\n" - " rtx_insn *%s ATTRIBUTE_UNUSED = safe_as_a (insn2_or_const0);\n"), - INSN2_PARAMETER_NAME); - fprintf (output_file, " switch (%s)\n {\n", INTERNAL_INSN_CODE_NAME); for (i = 0; i < description->decls_num; i++) if (description->decls[i]->mode == dm_insn_reserv @@ -8466,9 +8443,8 @@ output_internal_maximal_insn_latency_func (void) int i; int max; - fprintf (output_file, "static int\n%s (int %s ATTRIBUTE_UNUSED,\n\trtx %s ATTRIBUTE_UNUSED)\n", - "internal_maximal_insn_latency", INTERNAL_INSN_CODE_NAME, - INSN_PARAMETER_NAME); + fprintf (output_file, "static int\n%s (int %s ATTRIBUTE_UNUSED)\n", + "internal_maximal_insn_latency", INTERNAL_INSN_CODE_NAME); fprintf (output_file, "{\n"); if (DECL_INSN_RESERV (advance_cycle_insn_decl)->insn_num == 0) @@ -8505,7 +8481,7 @@ output_internal_maximal_insn_latency_func (void) static void output_insn_latency_func (void) { - fprintf (output_file, "int\n%s (rtx %s, rtx %s)\n", + fprintf (output_file, "int\n%s (rtx_insn *%s, rtx_insn *%s)\n", INSN_LATENCY_FUNC_NAME, INSN_PARAMETER_NAME, INSN2_PARAMETER_NAME); fprintf (output_file, "{\n int %s, %s;\n", INTERNAL_INSN_CODE_NAME, INTERNAL_INSN2_CODE_NAME); @@ -8523,15 +8499,14 @@ output_insn_latency_func (void) static void output_maximal_insn_latency_func (void) { - fprintf (output_file, "int\n%s (rtx %s)\n", + fprintf (output_file, "int\n%s (rtx_insn *%s)\n", "maximal_insn_latency", INSN_PARAMETER_NAME); fprintf (output_file, "{\n int %s;\n", INTERNAL_INSN_CODE_NAME); output_internal_insn_code_evaluation (INSN_PARAMETER_NAME, INTERNAL_INSN_CODE_NAME, 0); - fprintf (output_file, " return %s (%s, %s);\n}\n\n", - "internal_maximal_insn_latency", - INTERNAL_INSN_CODE_NAME, INSN_PARAMETER_NAME); + fprintf (output_file, " return %s (%s);\n}\n\n", + "internal_maximal_insn_latency", INTERNAL_INSN_CODE_NAME); } /* The function outputs PHR interface function `print_reservation'. */