On Mon, Feb 10, 2020 at 11:40 AM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 8:22 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > Since nested function isn't only called directly, there is ENDBR32 at
> > function entry and we need to skip it for direct jump in trampoline.
>
> Hm, I'm afraid I don't understand this comment. Can you perhaps rephrase it?
>

ix86_trampoline_init has

     /* Compute offset from the end of the jmp to the target function.
         In the case in which the trampoline stores the static chain on
         the stack, we need to skip the first insn which pushes the
         (call-saved) register static chain; this push is 1 byte.  */
      offset += 5;
      disp = expand_binop (SImode, sub_optab, fnaddr,
                           plus_constant (Pmode, XEXP (m_tramp, 0),
                                          offset - (MEM_P (chain) ? 1 : 0)),
                           NULL_RTX, 1, OPTAB_DIRECT);
      emit_move_insn (mem, disp);

Without CET, we got

0000011 <bar.1878>:
  11: 56                    push   %esi
  12: 55                    push   %ebp   <<<<<< trampoline jumps here.
  13: 89 e5                mov    %esp,%ebp
  15: 83 ec 08              sub    $0x8,%esp

With CET, if bar isn't only called directly, we got

00000015 <bar.1878>:
  15: f3 0f 1e fb          endbr32
  19: 56                    push   %esi
  1a: 55                    push   %ebp   <<<<<<<< trampoline jumps here.
  1b: 89 e5                mov    %esp,%ebp
  1d: 83 ec 08              sub    $0x8,%esp

We need to add 4 bytes for trampoline to skip endbr32.

Here is the updated patch to check if nested function isn't only
called directly,


-- 
H.J.
From 10ffeb41d1cdbd42f19ba08fdd6ce4a9913a5b5b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Mon, 10 Feb 2020 11:10:52 -0800
Subject: [PATCH] i386: Skip ENDBR32 at nested function entry

If nested function isn't only called directly, there is ENDBR32 at
function entry and we need to skip it for direct jump in trampoline.

Tested on Linux/x86-64 CET machine with and without -m32.

gcc/

	PR target/93656
	* config/i386/i386.c (ix86_trampoline_init): Skip ENDBR32 at
	nested function entry.

gcc/testsuite/

	PR target/93656
	* gcc.target/i386/pr93656.c: New test.
---
 gcc/config/i386/i386.c                  | 8 +++++++-
 gcc/testsuite/gcc.target/i386/pr93656.c | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93656.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 44bc0e0176a..bc494ec19b6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -16839,9 +16839,15 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
 	 the stack, we need to skip the first insn which pushes the
 	 (call-saved) register static chain; this push is 1 byte.  */
       offset += 5;
+      int skip = MEM_P (chain) ? 1 : 0;
+      /* If nested function isn't only called directly, there is ENDBR32
+	 at function entry and we need to skip it.  */
+      if (need_endbr
+	  && !cgraph_node::get (fndecl)->only_called_directly_p ())
+	skip += 4;
       disp = expand_binop (SImode, sub_optab, fnaddr,
 			   plus_constant (Pmode, XEXP (m_tramp, 0),
-					  offset - (MEM_P (chain) ? 1 : 0)),
+					  offset - skip),
 			   NULL_RTX, 1, OPTAB_DIRECT);
       emit_move_insn (mem, disp);
     }
diff --git a/gcc/testsuite/gcc.target/i386/pr93656.c b/gcc/testsuite/gcc.target/i386/pr93656.c
new file mode 100644
index 00000000000..f0ac8c8edaa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93656.c
@@ -0,0 +1,4 @@
+/* { dg-do run { target { ia32 && cet } } } */
+/* { dg-options "-O2 -fcf-protection" } */
+
+#include "pr67770.c"
-- 
2.24.1

Reply via email to