This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
[PATCH] clean up insn-automata.c (was: Re: out of bounds access in insn-automata.c)
- From: Alexander Monakov <amonakov at ispras dot ru>
- To: Bernd Schmidt <bschmidt at redhat dot com>
- Cc: Aldy Hernandez <aldyh at redhat dot com>, GCC Mailing List <gcc at gcc dot gnu dot org>, "Vladimir N. Makarov" <vmakarov at redhat dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Andrey Belevantsev <abel at ispras dot ru>
- Date: Wed, 11 May 2016 08:39:49 +0300 (MSK)
- Subject: [PATCH] clean up insn-automata.c (was: Re: out of bounds access in insn-automata.c)
- Authentication-results: sourceware.org; auth=none
- References: <56F23888 dot 5080506 at redhat dot com> <56F2B57E dot 6080300 at t-online dot de> <56F3BEA5 dot 1090007 at redhat dot com> <56F3ED30 dot 1050407 at redhat dot com> <alpine dot LNX dot 2 dot 20 dot 1603241729460 dot 13116 at monopod dot intra dot ispras dot ru> <56F4B3DE dot 8000708 at redhat dot com> <56FC2466 dot 1060406 at redhat dot com>
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 <rtx_insn *> (%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 <rtx_insn *> (%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 <rtx_insn *> (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 <rtx_insn *> (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'. */