This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Fix stack corruption in regrename
- To: gcc-patches at gcc dot gnu dot org
- Subject: Fix stack corruption in regrename
- From: Alexandre Oliva <aoliva at redhat dot com>
- Date: 31 Jul 2001 11:52:44 -0300
- Organization: GCC Team, Red Hat
The problem is that reload inserts some USEs of pseudos that are not
deleted nor replaced at the end of reload. Then, when regrename marks
them, it corrupts the stack, since it expects only hard regs to be
referenced. Unfortunately, the testcase I have is from a proprietary
testsuite, and it shows up on a port yet to be contributed. But it
appears that Bernd already ran into this problem and attempted to fix
it, using a different approach that broke mips.
The case I investigated, in which the USE was left over after reload,
was the case in which the pseudo was determined to be equivalent to a
constant, but reload decided it had to load the constant from a
constant pool entry into a hard register previously determined by
the register allocator.
The insn that initialized the constant pseudo, that appeared in its
reg_equiv_init, was deleted by reload. The pseudo remained
referenced, but not initialized, which is bad in itself. However, the
real problem was that the USE survived reload.
I'm not sure we shouldn't have refrained from emitting the USE in case
the register's reg_equiv_init is non-empty. I suppose the USE may
still be useful for reload inheritance purposes, so I decided to leave
it in, and mark all reload-emitted USEs for inheritance purposes with
a REG_EQUAL note pointing at the USEd value itself, since it's the
presence of the REG_EQUAL note in a USE insn that tells reload it's ok
to delete the USE insn, right after the following comment in reload():
/* Make a pass over all the insns and delete all USEs which we inserted
only to tag a REG_EQUAL note on them. [...]
Here's the patch I came up with. Only the changes in reload.c are
necessary to get the behavior correct; those in the other files are
just introducing sanity-checking.
The patch survived bootstrapping on i686-pc-linux-gnu, without
regressions in the testsuite. Ok to install? Ok for the 3.0 branch?
Index: gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
* regrename.c (note_sets, clear_dead_regs): Abort if pseudos are
encountered.
* reload1.c (reload_combine_note_use): Likewise, inside USEs and
CLOBBERs.
* reload.c (find_reloads): Emit REG_EQUAL note in USE insn.
(find_reloads_toplev, find_reloads_address, subst_reg_equivs,
find_reloads_subreg_address): Likewise.
Index: gcc/regrename.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/regrename.c,v
retrieving revision 1.23
diff -u -p -r1.23 regrename.c
--- gcc/regrename.c 2001/07/22 07:10:46 1.23
+++ gcc/regrename.c 2001/07/31 12:19:07
@@ -112,6 +112,11 @@ note_sets (x, set, data)
return;
regno = REGNO (x);
nregs = HARD_REGNO_NREGS (regno, GET_MODE (x));
+
+ /* There must not be pseudos at this point. */
+ if (regno + nregs > FIRST_PSEUDO_REGISTER)
+ abort ();
+
while (nregs-- > 0)
SET_HARD_REG_BIT (*pset, regno + nregs);
}
@@ -132,6 +137,11 @@ clear_dead_regs (pset, kind, notes)
rtx reg = XEXP (note, 0);
unsigned int regno = REGNO (reg);
int nregs = HARD_REGNO_NREGS (regno, GET_MODE (reg));
+
+ /* There must not be pseudos at this point. */
+ if (regno + nregs > FIRST_PSEUDO_REGISTER)
+ abort ();
+
while (nregs-- > 0)
CLEAR_HARD_REG_BIT (*pset, regno + nregs);
}
Index: gcc/reload.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload.c,v
retrieving revision 1.152
diff -u -p -r1.152 reload.c
--- gcc/reload.c 2001/07/23 22:41:10 1.152
+++ gcc/reload.c 2001/07/31 12:19:08
@@ -3815,7 +3815,12 @@ find_reloads (insn, replace, ind_levels,
if (GET_CODE (operand) == REG)
{
if (modified[i] != RELOAD_WRITE)
- emit_insn_before (gen_rtx_USE (VOIDmode, operand), insn);
+ /* We mark the USE with a REG_EQUAL note so that we
+ recognize it as one that can be safely deleted at
+ the end of reload. */
+ REG_NOTES (emit_insn_before (gen_rtx_USE (VOIDmode, operand),
+ insn))
+ = gen_rtx_EXPR_LIST (REG_EQUAL, operand, NULL_RTX);
if (modified[i] != RELOAD_READ)
emit_insn_after (gen_rtx_CLOBBER (VOIDmode, operand), insn);
}
@@ -4301,7 +4306,11 @@ find_reloads_toplev (x, opnum, type, ind
this substitution. We have to emit a USE of the pseudo so
that delete_output_reload can see it. */
if (replace_reloads && recog_data.operand[opnum] != x)
- emit_insn_before (gen_rtx_USE (VOIDmode, x), insn);
+ /* We mark the USE with a REG_EQUAL note so that we
+ recognize it as one that can be safely deleted at
+ the end of reload. */
+ REG_NOTES (emit_insn_before (gen_rtx_USE (VOIDmode, x), insn))
+ = gen_rtx_EXPR_LIST (REG_EQUAL, x, NULL_RTX);
x = mem;
i = find_reloads_address (GET_MODE (x), &x, XEXP (x, 0), &XEXP (x, 0),
opnum, type, ind_levels, insn);
@@ -4578,7 +4587,13 @@ find_reloads_address (mode, memrefloc, a
&& ! rtx_equal_p (tem, reg_equiv_mem[regno]))
{
*loc = tem;
- emit_insn_before (gen_rtx_USE (VOIDmode, ad), insn);
+ /* We mark the USE with a REG_EQUAL note so that
+ we recognize it as one that can be safely
+ deleted at the end of reload. */
+ REG_NOTES (emit_insn_before (gen_rtx_USE (VOIDmode, ad),
+ insn))
+ = gen_rtx_EXPR_LIST (REG_EQUAL, ad, NULL_RTX);
+
/* This doesn't really count as replacing the address
as a whole, since it is still a memory access. */
}
@@ -4909,7 +4924,11 @@ subst_reg_equivs (ad, insn)
if (! rtx_equal_p (mem, reg_equiv_mem[regno]))
{
subst_reg_equivs_changed = 1;
- emit_insn_before (gen_rtx_USE (VOIDmode, ad), insn);
+ /* We mark the USE with a REG_EQUAL note so that we
+ recognize it as one that can be safely deleted at
+ the end of reload. */
+ REG_NOTES (emit_insn_before (gen_rtx_USE (VOIDmode, ad), insn))
+ = gen_rtx_EXPR_LIST (REG_EQUAL, ad, NULL_RTX);
return mem;
}
}
@@ -5741,7 +5760,13 @@ find_reloads_subreg_address (x, force_re
this substitution. We have to emit a USE of the pseudo so
that delete_output_reload can see it. */
if (replace_reloads && recog_data.operand[opnum] != x)
- emit_insn_before (gen_rtx_USE (VOIDmode, SUBREG_REG (x)), insn);
+ /* We mark the USE with a REG_EQUAL note so that we
+ recognize it as one that can be safely deleted at
+ the end of reload. */
+ REG_NOTES (emit_insn_before (gen_rtx_USE (VOIDmode,
+ SUBREG_REG (x)),
+ insn))
+ = gen_rtx_EXPR_LIST (REG_EQUAL, SUBREG_REG (x), NULL_RTX);
x = tem;
}
}
Index: gcc/reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.278
diff -u -p -r1.278 reload1.c
--- gcc/reload1.c 2001/07/19 19:46:29 1.278
+++ gcc/reload1.c 2001/07/31 12:19:08
@@ -8972,7 +8972,12 @@ reload_combine_note_use (xp, insn)
case CLOBBER:
if (GET_CODE (SET_DEST (x)) == REG)
- return;
+ {
+ /* No spurious CLOBBERs of pseudo registers may remain. */
+ if (REGNO (SET_DEST (x)) >= FIRST_PSEUDO_REGISTER)
+ abort ();
+ return;
+ }
break;
case PLUS:
@@ -8989,10 +8994,9 @@ reload_combine_note_use (xp, insn)
int use_index;
int nregs;
- /* Some spurious USEs of pseudo registers might remain.
- Just ignore them. */
+ /* No spurious USEs of pseudo registers may remain. */
if (regno >= FIRST_PSEUDO_REGISTER)
- return;
+ abort ();
nregs = HARD_REGNO_NREGS (regno, GET_MODE (x));
--
Alexandre Oliva Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist *Please* write to mailing lists, not to me