head: MIPS: Workaround an R4000SC/MC branch/divide erratum
Richard Sandiford
rsandifo@redhat.com
Mon Mar 1 18:35:00 GMT 2004
"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:
> I'm not particularly happy about that sprintf() that does not prevent
> buffer overflows, but that's what's used elsewhere. The string passed to
> mips_output_division() is currently fixed in length, so no problem will
> appear unless that changes.
Yeah, I'd really rather avoid a static buffer if possible. I know
it's used in mips_output_conditional_branch(), but that's not really
a good example to follow.
> @@ -9121,20 +9174,27 @@ const char *
> mips_output_division (const char *division, rtx *operands)
> {
> const char *s = division;
> + static char buffer[30];
>
> if (TARGET_CHECK_ZERO_DIV)
> {
> - output_asm_insn (s, operands);
> +
> + if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
> + sprintf (buffer, "%%(%s%%)", division);
> + else
> + sprintf (buffer, "%s", division);
> + output_asm_insn (buffer, operands);
>
> if (TARGET_MIPS16)
> s = "bnez\t%2,1f\n\tbreak\t7\n1:";
> else
> s = "bne\t%2,%.,1f%#\n\tbreak\t7\n1:";
> }
> - if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
> + else if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
> {
> - output_asm_insn (s, operands);
> - s = "nop";
> + sprintf (buffer, "%%(%s", division);
> + output_asm_insn (buffer, operands);
> + s = "nop%)";
> }
> return s;
> }
I'm not too keen on the "sprintf (..., "%s", ...)" bit. Better to just
call output_asm_insn in the TARGET_FIX* arm.
Anyhow, after the change to force division into the delay slot,
I think all we need to do is swap the clauses around, like so:
*************** mips_output_division (const char *divisi
*** 9223,9228 ****
--- 9223,9233 ----
const char *s;
s = division;
+ if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
+ {
+ output_asm_insn (s, operands);
+ s = "nop";
+ }
if (TARGET_CHECK_ZERO_DIV)
{
if (TARGET_MIPS16)
*************** mips_output_division (const char *divisi
*** 9236,9246 ****
output_asm_insn (s, operands);
s = "break\t7%)\n1:";
}
- }
- if (TARGET_FIX_R4000)
- {
- output_asm_insn (s, operands);
- s = "nop";
}
return s;
}
--- 9241,9246 ----
If you agree, I'll test your patch with that change.
Richard
More information about the Gcc-patches
mailing list