x86: Restore the frame pointer in word_mode

Message ID 20200412212811.3898988-1-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Restore the frame pointer in word_mode
Related show

Commit Message

Richard Biener via Gcc-patches April 12, 2020, 9:28 p.m.
We must restore the frame pointer in word_mode for eh_return epilogues
since the upper 32 bits of RBP register can have any values.

Tested on Linux/x32 and Linux/x86-64.  OK for master and backport to
GCC 8/9 branches?

Thanks.

H.J.
---
	PR target/94556
	* config/i386/i386.c (ix86_expand_epilogue): Restore the frame
	pointer in word_mode for eh_return epilogues.
---
 gcc/config/i386/i386.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
2.25.2

Comments

Richard Biener via Gcc-patches April 13, 2020, 8:41 a.m. | #1
On Sun, Apr 12, 2020 at 11:28 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>

> We must restore the frame pointer in word_mode for eh_return epilogues

> since the upper 32 bits of RBP register can have any values.

>

> Tested on Linux/x32 and Linux/x86-64.  OK for master and backport to

> GCC 8/9 branches?

>

> Thanks.

>

> H.J.

> ---

>         PR target/94556

>         * config/i386/i386.c (ix86_expand_epilogue): Restore the frame

>         pointer in word_mode for eh_return epilogues.

> ---

>  gcc/config/i386/i386.c | 12 +++++++++---

>  1 file changed, 9 insertions(+), 3 deletions(-)

>

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c

> index ca3b7dc06c2..f9c8f75b559 100644

> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -9052,8 +9052,14 @@ ix86_expand_epilogue (int style)

>               t = plus_constant (Pmode, t, m->fs.fp_offset - UNITS_PER_WORD);

>               emit_insn (gen_rtx_SET (sa, t));

>

> -             t = gen_frame_mem (Pmode, hard_frame_pointer_rtx);

> -             insn = emit_move_insn (hard_frame_pointer_rtx, t);

> +             /* NB: eh_return epilogues must restore the frame pointer

> +                in word_mode since the upper 32 bits of RBP register

> +                can have any values.  */

> +             t = gen_frame_mem (word_mode, hard_frame_pointer_rtx);

> +             rtx frame_reg = hard_frame_pointer_rtx;

> +             if (Pmode != word_mode)

> +               frame_reg = gen_rtx_REG (word_mode, REGNO (frame_reg));

> +             insn = emit_move_insn (frame_reg, t);


I didn't test it myself, but it looks to me that you can use

"rtx frame_reg = gen_rtx_REG (word_mode, HARD_FRAME_POINTER_REGNUM);"

unconditionally, since gen_rtx_REG has some bypass code to return
hard_frame_pointer_rtx when appropriate.

I don't know x32 enough for a functional review, so LGTM with the above change.

Please wait a week before backporting the patch.

Thanks,
Uros.

>               /* Note that we use SA as a temporary CFA, as the return

>                  address is at the proper place relative to it.  We

> @@ -9068,7 +9074,7 @@ ix86_expand_epilogue (int style)

>               add_reg_note (insn, REG_CFA_DEF_CFA,

>                             plus_constant (Pmode, sa, UNITS_PER_WORD));

>               ix86_add_queued_cfa_restore_notes (insn);

> -             add_reg_note (insn, REG_CFA_RESTORE, hard_frame_pointer_rtx);

> +             add_reg_note (insn, REG_CFA_RESTORE, frame_reg);

>               RTX_FRAME_RELATED_P (insn) = 1;

>

>               m->fs.cfa_reg = sa;

> --

> 2.25.2

>

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ca3b7dc06c2..f9c8f75b559 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -9052,8 +9052,14 @@  ix86_expand_epilogue (int style)
 	      t = plus_constant (Pmode, t, m->fs.fp_offset - UNITS_PER_WORD);
 	      emit_insn (gen_rtx_SET (sa, t));
 
-	      t = gen_frame_mem (Pmode, hard_frame_pointer_rtx);
-	      insn = emit_move_insn (hard_frame_pointer_rtx, t);
+	      /* NB: eh_return epilogues must restore the frame pointer
+		 in word_mode since the upper 32 bits of RBP register
+		 can have any values.  */
+	      t = gen_frame_mem (word_mode, hard_frame_pointer_rtx);
+	      rtx frame_reg = hard_frame_pointer_rtx;
+	      if (Pmode != word_mode)
+		frame_reg = gen_rtx_REG (word_mode, REGNO (frame_reg));
+	      insn = emit_move_insn (frame_reg, t);
 
 	      /* Note that we use SA as a temporary CFA, as the return
 		 address is at the proper place relative to it.  We
@@ -9068,7 +9074,7 @@  ix86_expand_epilogue (int style)
 	      add_reg_note (insn, REG_CFA_DEF_CFA,
 			    plus_constant (Pmode, sa, UNITS_PER_WORD));
 	      ix86_add_queued_cfa_restore_notes (insn);
-	      add_reg_note (insn, REG_CFA_RESTORE, hard_frame_pointer_rtx);
+	      add_reg_note (insn, REG_CFA_RESTORE, frame_reg);
 	      RTX_FRAME_RELATED_P (insn) = 1;
 
 	      m->fs.cfa_reg = sa;