public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
@ 2020-05-02 11:55 H.J. Lu
  2020-05-22 11:22 ` PING: " H.J. Lu
  2020-06-12  3:24 ` Jeff Law
  0 siblings, 2 replies; 9+ messages in thread
From: H.J. Lu @ 2020-05-02 11:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: Uros Bizjak, Jeff Law, Richard Biener, Jakub Jelinek

Currently patchable area is at the wrong place.  It is placed immediately
after function label, before both .cfi_startproc and ENDBR.  This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
changes ENDBR insertion pass to also insert patchable area instruction.
TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
patchable area before .cfi_startproc and ENDBR.

OK for master?

Thanks.

H.J.
---
gcc/

	PR target/93492
	* config/i386/i386-features.c (rest_of_insert_endbranch):
	Renamed to ...
	(rest_of_insert_endbr_and_patchable_area): Change return type
	to void. Add need_endbr and patchable_area_size arguments.
	Don't call timevar_push nor timevar_pop.  Replace
	endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
	UNSPECV_PATCHABLE_AREA for patchable area.
	(pass_data_insert_endbranch): Renamed to ...
	(pass_data_insert_endbr_and_patchable_area): This.  Change
	pass name to endbr_and_patchable_area.
	(pass_insert_endbranch): Renamed to ...
	(pass_insert_endbr_and_patchable_area): This.  Add need_endbr
	and patchable_area_size;.
	(pass_insert_endbr_and_patchable_area::gate): Set and check
	need_endbr and patchable_area_size.
	(pass_insert_endbr_and_patchable_area::execute): Call
	timevar_push and timevar_pop.  Pass need_endbr and
	patchable_area_size to rest_of_insert_endbr_and_patchable_area.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386-passes.def: Replace pass_insert_endbranch
	with pass_insert_endbr_and_patchable_area.
	* config/i386/i386-protos.h (ix86_output_patchable_area): New.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386.c (ix86_asm_output_function_label): Set
	function_label_emitted to true.
	(ix86_print_patchable_function_entry): New function.
	(ix86_output_patchable_area): Likewise.
	(x86_function_profiler): Replace endbr_queued_at_entrance with
	insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
	Call ix86_output_patchable_area to generate patchable area if
	needed.
	(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
	* i386.h (queued_insn_type): New.
	(machine_function): Add function_label_emitted.  Replace
	endbr_queued_at_entrance with insn_queued_at_entrance.
	* config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
	(patchable_area): New.

gcc/testsuite/

	PR target/93492
	* gcc.target/i386/pr93492-1.c: New test.
	* gcc.target/i386/pr93492-2.c: Likewise.
	* gcc.target/i386/pr93492-3.c: Likewise.
	* gcc.target/i386/pr93492-4.c: Likewise.
	* gcc.target/i386/pr93492-5.c: Likewise.
---
 gcc/config/i386/i386-features.c           | 142 ++++++++++++++--------
 gcc/config/i386/i386-passes.def           |   2 +-
 gcc/config/i386/i386-protos.h             |   5 +-
 gcc/config/i386/i386.c                    |  51 +++++++-
 gcc/config/i386/i386.h                    |  14 ++-
 gcc/config/i386/i386.md                   |  17 +++
 gcc/testsuite/gcc.target/i386/pr93492-1.c |  73 +++++++++++
 gcc/testsuite/gcc.target/i386/pr93492-2.c |  12 ++
 gcc/testsuite/gcc.target/i386/pr93492-3.c |  13 ++
 gcc/testsuite/gcc.target/i386/pr93492-4.c |  11 ++
 gcc/testsuite/gcc.target/i386/pr93492-5.c |  12 ++
 11 files changed, 296 insertions(+), 56 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-5.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 78fb373db6e..41cc8b583b6 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1941,48 +1941,83 @@ make_pass_stv (gcc::context *ctxt)
   return new pass_stv (ctxt);
 }
 
-/* Inserting ENDBRANCH instructions.  */
+/* Inserting ENDBR and pseudo patchable-area instructions.  */
 
-static unsigned int
-rest_of_insert_endbranch (void)
+static void
+rest_of_insert_endbr_and_patchable_area (bool need_endbr,
+					 unsigned int patchable_area_size)
 {
-  timevar_push (TV_MACH_DEP);
-
-  rtx cet_eb;
+  rtx endbr;
   rtx_insn *insn;
+  rtx_insn *endbr_insn = NULL;
   basic_block bb;
 
-  /* Currently emit EB if it's a tracking function, i.e. 'nocf_check' is
-     absent among function attributes.  Later an optimization will be
-     introduced to make analysis if an address of a static function is
-     taken.  A static function whose address is not taken will get a
-     nocf_check attribute.  This will allow to reduce the number of EB.  */
-
-  if (!lookup_attribute ("nocf_check",
-			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
-      && (!flag_manual_endbr
-	  || lookup_attribute ("cf_check",
-			       DECL_ATTRIBUTES (cfun->decl)))
-      && (!cgraph_node::get (cfun->decl)->only_called_directly_p ()
-	  || ix86_cmodel == CM_LARGE
-	  || ix86_cmodel == CM_LARGE_PIC
-	  || flag_force_indirect_call
-	  || (TARGET_DLLIMPORT_DECL_ATTRIBUTES
-	      && DECL_DLLIMPORT_P (cfun->decl))))
-    {
-      /* Queue ENDBR insertion to x86_function_profiler.  */
+  if (need_endbr)
+    {
+      /* Currently emit EB if it's a tracking function, i.e. 'nocf_check'
+	 is absent among function attributes.  Later an optimization will
+	 be introduced to make analysis if an address of a static function
+	 is taken.  A static function whose address is not taken will get
+	 a nocf_check attribute.  This will allow to reduce the number of
+	 EB.  */
+      if (!lookup_attribute ("nocf_check",
+			     TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+	  && (!flag_manual_endbr
+	      || lookup_attribute ("cf_check",
+				   DECL_ATTRIBUTES (cfun->decl)))
+	  && (!cgraph_node::get (cfun->decl)->only_called_directly_p ()
+	      || ix86_cmodel == CM_LARGE
+	      || ix86_cmodel == CM_LARGE_PIC
+	      || flag_force_indirect_call
+	      || (TARGET_DLLIMPORT_DECL_ATTRIBUTES
+		  && DECL_DLLIMPORT_P (cfun->decl))))
+	{
+	  if (crtl->profile && flag_fentry)
+	    {
+	      /* Queue ENDBR insertion to x86_function_profiler.
+		 NB: Any patchable-area insn will be inserted after
+		 ENDBR.  */
+	      cfun->machine->insn_queued_at_entrance = TYPE_ENDBR;
+	    }
+	  else
+	    {
+	      endbr = gen_nop_endbr ();
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      rtx_insn *insn = BB_HEAD (bb);
+	      endbr_insn = emit_insn_before (endbr, insn);
+	    }
+	}
+    }
+
+  if (patchable_area_size)
+    {
       if (crtl->profile && flag_fentry)
-	cfun->machine->endbr_queued_at_entrance = true;
+	{
+	  /* Queue patchable-area insertion to x86_function_profiler.
+	     NB: If there is a queued ENDBR, x86_function_profiler
+	     will also handle patchable-area.  */
+	  if (!cfun->machine->insn_queued_at_entrance)
+	    cfun->machine->insn_queued_at_entrance = TYPE_PATCHABLE_AREA;
+	}
       else
 	{
-	  cet_eb = gen_nop_endbr ();
-
-	  bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
-	  insn = BB_HEAD (bb);
-	  emit_insn_before (cet_eb, insn);
+	  rtx patchable_area
+	    = gen_patchable_area (GEN_INT (patchable_area_size),
+				  GEN_INT (crtl->patch_area_entry == 0));
+	  if (endbr_insn)
+	    emit_insn_after (patchable_area, endbr_insn);
+	  else
+	    {
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      insn = BB_HEAD (bb);
+	      emit_insn_before (patchable_area, insn);
+	    }
 	}
     }
 
+  if (!need_endbr)
+    return;
+
   bb = 0;
   FOR_EACH_BB_FN (bb, cfun)
     {
@@ -1991,7 +2026,6 @@ rest_of_insert_endbranch (void)
 	{
 	  if (CALL_P (insn))
 	    {
-	      bool need_endbr;
 	      need_endbr = find_reg_note (insn, REG_SETJMP, NULL) != NULL;
 	      if (!need_endbr && !SIBLING_CALL_P (insn))
 		{
@@ -2022,8 +2056,8 @@ rest_of_insert_endbranch (void)
 	      /* Generate ENDBRANCH after CALL, which can return more than
 		 twice, setjmp-like functions.  */
 
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after_setloc (cet_eb, insn, INSN_LOCATION (insn));
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after_setloc (endbr, insn, INSN_LOCATION (insn));
 	      continue;
 	    }
 
@@ -2053,31 +2087,30 @@ rest_of_insert_endbranch (void)
 		  dest_blk = e->dest;
 		  insn = BB_HEAD (dest_blk);
 		  gcc_assert (LABEL_P (insn));
-		  cet_eb = gen_nop_endbr ();
-		  emit_insn_after (cet_eb, insn);
+		  endbr = gen_nop_endbr ();
+		  emit_insn_after (endbr, insn);
 		}
 	      continue;
 	    }
 
 	  if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
 	    {
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after (cet_eb, insn);
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after (endbr, insn);
 	      continue;
 	    }
 	}
     }
 
-  timevar_pop (TV_MACH_DEP);
-  return 0;
+  return;
 }
 
 namespace {
 
-const pass_data pass_data_insert_endbranch =
+const pass_data pass_data_insert_endbr_and_patchable_area =
 {
   RTL_PASS, /* type.  */
-  "cet", /* name.  */
+  "endbr_and_patchable_area", /* name.  */
   OPTGROUP_NONE, /* optinfo_flags.  */
   TV_MACH_DEP, /* tv_id.  */
   0, /* properties_required.  */
@@ -2087,32 +2120,41 @@ const pass_data pass_data_insert_endbranch =
   0, /* todo_flags_finish.  */
 };
 
-class pass_insert_endbranch : public rtl_opt_pass
+class pass_insert_endbr_and_patchable_area : public rtl_opt_pass
 {
 public:
-  pass_insert_endbranch (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_insert_endbranch, ctxt)
+  pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
+    : rtl_opt_pass (pass_data_insert_endbr_and_patchable_area, ctxt)
   {}
 
   /* opt_pass methods: */
   virtual bool gate (function *)
     {
-      return ((flag_cf_protection & CF_BRANCH));
+      need_endbr = (flag_cf_protection & CF_BRANCH) != 0;
+      patchable_area_size = crtl->patch_area_size - crtl->patch_area_entry;
+      return need_endbr || patchable_area_size;
     }
 
   virtual unsigned int execute (function *)
     {
-      return rest_of_insert_endbranch ();
+      timevar_push (TV_MACH_DEP);
+      rest_of_insert_endbr_and_patchable_area (need_endbr,
+					       patchable_area_size);
+      timevar_pop (TV_MACH_DEP);
+      return 0;
     }
 
-}; // class pass_insert_endbranch
+private:
+  bool need_endbr;
+  unsigned int patchable_area_size;
+}; // class pass_insert_endbr_and_patchable_area
 
 } // anon namespace
 
 rtl_opt_pass *
-make_pass_insert_endbranch (gcc::context *ctxt)
+make_pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
 {
-  return new pass_insert_endbranch (ctxt);
+  return new pass_insert_endbr_and_patchable_area (ctxt);
 }
 
 /* At entry of the nearest common dominator for basic blocks with
diff --git a/gcc/config/i386/i386-passes.def b/gcc/config/i386/i386-passes.def
index 41386a13d88..d83c7b956b1 100644
--- a/gcc/config/i386/i386-passes.def
+++ b/gcc/config/i386/i386-passes.def
@@ -30,6 +30,6 @@ along with GCC; see the file COPYING3.  If not see
      CONSTM1_RTX generated by the STV pass can be CSEed.  */
   INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
 
-  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbranch);
+  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbr_and_patchable_area);
 
   INSERT_PASS_AFTER (pass_combine, 1, pass_remove_partial_avx_dependency);
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 39fcaa0ad5f..e5574496bb7 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -89,6 +89,8 @@ extern const char *output_fp_compare (rtx_insn *, rtx*, bool, bool);
 extern const char *output_adjust_stack_and_probe (rtx);
 extern const char *output_probe_stack_range (rtx, rtx);
 
+extern void ix86_output_patchable_area (unsigned int, bool);
+
 extern void ix86_expand_clear (rtx);
 extern void ix86_expand_move (machine_mode, rtx[]);
 extern void ix86_expand_vector_move (machine_mode, rtx[]);
@@ -378,6 +380,7 @@ class rtl_opt_pass;
 
 extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
 extern rtl_opt_pass *make_pass_stv (gcc::context *);
-extern rtl_opt_pass *make_pass_insert_endbranch (gcc::context *);
+extern rtl_opt_pass *make_pass_insert_endbr_and_patchable_area
+  (gcc::context *);
 extern rtl_opt_pass *make_pass_remove_partial_avx_dependency
   (gcc::context *);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index c4a538ed0c8..bdfe149e39e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1562,6 +1562,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
 {
   bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
 
+  if (cfun)
+    cfun->machine->function_label_emitted = true;
+
   if (is_ms_hook)
     {
       int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9368,6 +9371,38 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
     }
 }
 
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY.  */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+				     unsigned HOST_WIDE_INT patch_area_size,
+				     bool record_p)
+{
+  if (cfun->machine->function_label_emitted)
+    {
+      /* NB: When ix86_print_patchable_function_entry is called after
+	 function table has been emitted, we have inserted or queued
+	 a pseudo UNSPECV_PATCHABLE_AREA instruction at the proper
+	 place.  There is nothing to do here.  */
+      return;
+    }
+
+  default_print_patchable_function_entry (file, patch_area_size,
+					  record_p);
+}
+
+/* Output patchable area.  NB: default_print_patchable_function_entry
+   isn't available in i386.md.  */
+
+void
+ix86_output_patchable_area (unsigned int patch_area_size,
+			    bool record_p)
+{
+  default_print_patchable_function_entry (asm_out_file,
+					  patch_area_size,
+					  record_p);
+}
+
 /* Return a scratch register to use in the split stack prologue.  The
    split stack prologue is used for -fsplit-stack.  It is the first
    instructions in the function, even before the regular prologue.
@@ -20415,8 +20450,16 @@ current_fentry_section (const char **name)
 void
 x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
 {
-  if (cfun->machine->endbr_queued_at_entrance)
-    fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+  if (cfun->machine->insn_queued_at_entrance)
+    {
+      if (cfun->machine->insn_queued_at_entrance == TYPE_ENDBR)
+	fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+      unsigned int patch_area_size
+	= crtl->patch_area_size - crtl->patch_area_entry;
+      if (patch_area_size)
+	ix86_output_patchable_area (patch_area_size,
+				    crtl->patch_area_entry == 0);
+    }
 
   const char *mcount_name = MCOUNT_NAME;
 
@@ -23013,6 +23056,10 @@ ix86_run_selftests (void)
 #undef TARGET_ASM_FUNCTION_EPILOGUE
 #define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  ix86_print_patchable_function_entry
+
 #undef TARGET_ENCODE_SECTION_INFO
 #ifndef SUBTARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 08245f64322..77575875192 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2757,6 +2757,13 @@ enum function_type
   TYPE_EXCEPTION
 };
 
+enum queued_insn_type
+{
+  TYPE_NONE = 0,
+  TYPE_ENDBR,
+  TYPE_PATCHABLE_AREA
+};
+
 struct GTY(()) machine_function {
   struct stack_local_entry *stack_locals;
   int varargs_gpr_size;
@@ -2847,8 +2854,11 @@ struct GTY(()) machine_function {
   /* Nonzero if the function places outgoing arguments on stack.  */
   BOOL_BITFIELD outgoing_args_on_stack : 1;
 
-  /* If true, ENDBR is queued at function entrance.  */
-  BOOL_BITFIELD endbr_queued_at_entrance : 1;
+  /* If true, ENDBR or patchable area is queued at function entrance.  */
+  ENUM_BITFIELD(queued_insn_type) insn_queued_at_entrance : 2;
+
+  /* If true, the function label has been emitted.  */
+  BOOL_BITFIELD function_label_emitted : 1;
 
   /* True if the function needs a stack frame.  */
   BOOL_BITFIELD stack_frame_required : 1;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b426c21d3dd..f7ca55ab4de 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -300,6 +300,9 @@ (define_c_enum "unspecv" [
   ;; For ENQCMD and ENQCMDS support
   UNSPECV_ENQCMD
   UNSPECV_ENQCMDS
+
+  ;; For patchable area support
+  UNSPECV_PATCHABLE_AREA
 ])
 
 ;; Constants to represent rounding modes in the ROUND instruction
@@ -21135,6 +21138,20 @@ (define_insn "speculation_barrier"
   [(set_attr "type" "other")
    (set_attr "length" "3")])
 
+(define_insn "patchable_area"
+  [(unspec_volatile [(match_operand 0 "const_int_operand")
+		     (match_operand 1 "const_int_operand")]
+		    UNSPECV_PATCHABLE_AREA)]
+  ""
+{
+  ix86_output_patchable_area (INTVAL (operands[0]),
+			      INTVAL (operands[1]) != 0);
+  return "";
+}
+  [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))
+   (set_attr "length_immediate" "0")
+   (set_attr "modrm" "0")])
+
 (include "mmx.md")
 (include "sse.md")
 (include "sync.md")
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-1.c b/gcc/testsuite/gcc.target/i386/pr93492-1.c
new file mode 100644
index 00000000000..f978d2e5220
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-1.c
@@ -0,0 +1,73 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* Note: this test only checks the instructions in the function bodies,
+   not the placement of the patch label or nops before the function.  */
+
+/*
+**f10_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 0)))
+f10_none (void)
+{
+}
+
+/*
+**f10_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/*
+**f11_none:
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 1)))
+f11_none (void)
+{
+}
+
+/*
+**f11_endbr:
+**	endbr(32|64)
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 1)))
+f11_endbr (void)
+{
+}
+
+/*
+**f21_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (2, 1)))
+f21_none (void)
+{
+}
+
+/*
+**f21_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (2, 1)))
+f21_endbr (void)
+{
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-2.c b/gcc/testsuite/gcc.target/i386/pr93492-2.c
new file mode 100644
index 00000000000..ec26d4cc367
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-2.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-3.c b/gcc/testsuite/gcc.target/i386/pr93492-3.c
new file mode 100644
index 00000000000..1f03c627120
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-3.c
@@ -0,0 +1,13 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-4.c b/gcc/testsuite/gcc.target/i386/pr93492-4.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-4.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-5.c b/gcc/testsuite/gcc.target/i386/pr93492-5.c
new file mode 100644
index 00000000000..d04077c6007
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-5.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* PING: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-05-02 11:55 [PATCH] x86: Add UNSPECV_PATCHABLE_AREA H.J. Lu
@ 2020-05-22 11:22 ` H.J. Lu
  2020-06-09 16:34   ` PING^2: " H.J. Lu
  2020-06-12  3:24 ` Jeff Law
  1 sibling, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2020-05-22 11:22 UTC (permalink / raw)
  To: GCC Patches; +Cc: Uros Bizjak, Jeff Law, Richard Biener, Jakub Jelinek

On Sat, May 2, 2020 at 4:55 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Currently patchable area is at the wrong place.  It is placed immediately
> after function label, before both .cfi_startproc and ENDBR.  This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> changes ENDBR insertion pass to also insert patchable area instruction.
> TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> patchable area before .cfi_startproc and ENDBR.
>
> OK for master?
>
> Thanks.
>
> H.J.
> ---
> gcc/
>
>         PR target/93492
>         * config/i386/i386-features.c (rest_of_insert_endbranch):
>         Renamed to ...
>         (rest_of_insert_endbr_and_patchable_area): Change return type
>         to void. Add need_endbr and patchable_area_size arguments.
>         Don't call timevar_push nor timevar_pop.  Replace
>         endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
>         UNSPECV_PATCHABLE_AREA for patchable area.
>         (pass_data_insert_endbranch): Renamed to ...
>         (pass_data_insert_endbr_and_patchable_area): This.  Change
>         pass name to endbr_and_patchable_area.
>         (pass_insert_endbranch): Renamed to ...
>         (pass_insert_endbr_and_patchable_area): This.  Add need_endbr
>         and patchable_area_size;.
>         (pass_insert_endbr_and_patchable_area::gate): Set and check
>         need_endbr and patchable_area_size.
>         (pass_insert_endbr_and_patchable_area::execute): Call
>         timevar_push and timevar_pop.  Pass need_endbr and
>         patchable_area_size to rest_of_insert_endbr_and_patchable_area.
>         (make_pass_insert_endbranch): Renamed to ...
>         (make_pass_insert_endbr_and_patchable_area): This.
>         * config/i386/i386-passes.def: Replace pass_insert_endbranch
>         with pass_insert_endbr_and_patchable_area.
>         * config/i386/i386-protos.h (ix86_output_patchable_area): New.
>         (make_pass_insert_endbranch): Renamed to ...
>         (make_pass_insert_endbr_and_patchable_area): This.
>         * config/i386/i386.c (ix86_asm_output_function_label): Set
>         function_label_emitted to true.
>         (ix86_print_patchable_function_entry): New function.
>         (ix86_output_patchable_area): Likewise.
>         (x86_function_profiler): Replace endbr_queued_at_entrance with
>         insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
>         Call ix86_output_patchable_area to generate patchable area if
>         needed.
>         (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
>         * i386.h (queued_insn_type): New.
>         (machine_function): Add function_label_emitted.  Replace
>         endbr_queued_at_entrance with insn_queued_at_entrance.
>         * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
>         (patchable_area): New.
>
> gcc/testsuite/
>
>         PR target/93492
>         * gcc.target/i386/pr93492-1.c: New test.
>         * gcc.target/i386/pr93492-2.c: Likewise.
>         * gcc.target/i386/pr93492-3.c: Likewise.
>         * gcc.target/i386/pr93492-4.c: Likewise.
>         * gcc.target/i386/pr93492-5.c: Likewise.

PING:

https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545021.html

-- 
H.J.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* PING^2: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-05-22 11:22 ` PING: " H.J. Lu
@ 2020-06-09 16:34   ` H.J. Lu
  2020-06-16 14:17     ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2020-06-09 16:34 UTC (permalink / raw)
  To: GCC Patches, Jan Hubicka; +Cc: Jeff Law, Richard Biener, Jakub Jelinek

On Fri, May 22, 2020 at 4:22 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, May 2, 2020 at 4:55 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > Currently patchable area is at the wrong place.  It is placed immediately
> > after function label, before both .cfi_startproc and ENDBR.  This patch
> > adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> > changes ENDBR insertion pass to also insert patchable area instruction.
> > TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> > patchable area before .cfi_startproc and ENDBR.
> >
> > OK for master?
> >
> > Thanks.
> >
> > H.J.
> > ---
> > gcc/
> >
> >         PR target/93492
> >         * config/i386/i386-features.c (rest_of_insert_endbranch):
> >         Renamed to ...
> >         (rest_of_insert_endbr_and_patchable_area): Change return type
> >         to void. Add need_endbr and patchable_area_size arguments.
> >         Don't call timevar_push nor timevar_pop.  Replace
> >         endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
> >         UNSPECV_PATCHABLE_AREA for patchable area.
> >         (pass_data_insert_endbranch): Renamed to ...
> >         (pass_data_insert_endbr_and_patchable_area): This.  Change
> >         pass name to endbr_and_patchable_area.
> >         (pass_insert_endbranch): Renamed to ...
> >         (pass_insert_endbr_and_patchable_area): This.  Add need_endbr
> >         and patchable_area_size;.
> >         (pass_insert_endbr_and_patchable_area::gate): Set and check
> >         need_endbr and patchable_area_size.
> >         (pass_insert_endbr_and_patchable_area::execute): Call
> >         timevar_push and timevar_pop.  Pass need_endbr and
> >         patchable_area_size to rest_of_insert_endbr_and_patchable_area.
> >         (make_pass_insert_endbranch): Renamed to ...
> >         (make_pass_insert_endbr_and_patchable_area): This.
> >         * config/i386/i386-passes.def: Replace pass_insert_endbranch
> >         with pass_insert_endbr_and_patchable_area.
> >         * config/i386/i386-protos.h (ix86_output_patchable_area): New.
> >         (make_pass_insert_endbranch): Renamed to ...
> >         (make_pass_insert_endbr_and_patchable_area): This.
> >         * config/i386/i386.c (ix86_asm_output_function_label): Set
> >         function_label_emitted to true.
> >         (ix86_print_patchable_function_entry): New function.
> >         (ix86_output_patchable_area): Likewise.
> >         (x86_function_profiler): Replace endbr_queued_at_entrance with
> >         insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
> >         Call ix86_output_patchable_area to generate patchable area if
> >         needed.
> >         (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
> >         * i386.h (queued_insn_type): New.
> >         (machine_function): Add function_label_emitted.  Replace
> >         endbr_queued_at_entrance with insn_queued_at_entrance.
> >         * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
> >         (patchable_area): New.
> >
> > gcc/testsuite/
> >
> >         PR target/93492
> >         * gcc.target/i386/pr93492-1.c: New test.
> >         * gcc.target/i386/pr93492-2.c: Likewise.
> >         * gcc.target/i386/pr93492-3.c: Likewise.
> >         * gcc.target/i386/pr93492-4.c: Likewise.
> >         * gcc.target/i386/pr93492-5.c: Likewise.
>
> PING:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545021.html

PING.

-- 
H.J.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-05-02 11:55 [PATCH] x86: Add UNSPECV_PATCHABLE_AREA H.J. Lu
  2020-05-22 11:22 ` PING: " H.J. Lu
@ 2020-06-12  3:24 ` Jeff Law
  2020-06-12 12:56   ` [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED H.J. Lu
  1 sibling, 1 reply; 9+ messages in thread
From: Jeff Law @ 2020-06-12  3:24 UTC (permalink / raw)
  To: H.J. Lu, gcc-patches; +Cc: Uros Bizjak, Richard Biener, Jakub Jelinek

On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote:
> Currently patchable area is at the wrong place.  It is placed immediately
> after function label, before both .cfi_startproc and ENDBR.  This patch
> adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> changes ENDBR insertion pass to also insert patchable area instruction.
> TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> patchable area before .cfi_startproc and ENDBR.
> 
> OK for master?
> 
> Thanks.
> 
> H.J.
> ---
> gcc/
> 
> 	PR target/93492
> 	* config/i386/i386-features.c (rest_of_insert_endbranch):
> 	Renamed to ...
> 	(rest_of_insert_endbr_and_patchable_area): Change return type
> 	to void. Add need_endbr and patchable_area_size arguments.
> 	Don't call timevar_push nor timevar_pop.  Replace
> 	endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
> 	UNSPECV_PATCHABLE_AREA for patchable area.
> 	(pass_data_insert_endbranch): Renamed to ...
> 	(pass_data_insert_endbr_and_patchable_area): This.  Change
> 	pass name to endbr_and_patchable_area.
> 	(pass_insert_endbranch): Renamed to ...
> 	(pass_insert_endbr_and_patchable_area): This.  Add need_endbr
> 	and patchable_area_size;.
> 	(pass_insert_endbr_and_patchable_area::gate): Set and check
> 	need_endbr and patchable_area_size.
> 	(pass_insert_endbr_and_patchable_area::execute): Call
> 	timevar_push and timevar_pop.  Pass need_endbr and
> 	patchable_area_size to rest_of_insert_endbr_and_patchable_area.
> 	(make_pass_insert_endbranch): Renamed to ...
> 	(make_pass_insert_endbr_and_patchable_area): This.
> 	* config/i386/i386-passes.def: Replace pass_insert_endbranch
> 	with pass_insert_endbr_and_patchable_area.
> 	* config/i386/i386-protos.h (ix86_output_patchable_area): New.
> 	(make_pass_insert_endbranch): Renamed to ...
> 	(make_pass_insert_endbr_and_patchable_area): This.
> 	* config/i386/i386.c (ix86_asm_output_function_label): Set
> 	function_label_emitted to true.
> 	(ix86_print_patchable_function_entry): New function.
> 	(ix86_output_patchable_area): Likewise.
> 	(x86_function_profiler): Replace endbr_queued_at_entrance with
> 	insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
> 	Call ix86_output_patchable_area to generate patchable area if
> 	needed.
> 	(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
> 	* i386.h (queued_insn_type): New.
> 	(machine_function): Add function_label_emitted.  Replace
> 	endbr_queued_at_entrance with insn_queued_at_entrance.
> 	* config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
> 	(patchable_area): New.
> 
> gcc/testsuite/
> 
> 	PR target/93492
> 	* gcc.target/i386/pr93492-1.c: New test.
> 	* gcc.target/i386/pr93492-2.c: Likewise.
> 	* gcc.target/i386/pr93492-3.c: Likewise.
> 	* gcc.target/i386/pr93492-4.c: Likewise.
> 	* gcc.target/i386/pr93492-5.c: Likewise.
OK
jeff
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED
  2020-06-12  3:24 ` Jeff Law
@ 2020-06-12 12:56   ` H.J. Lu
  2020-06-26 22:11     ` Jeff Law
  0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2020-06-12 12:56 UTC (permalink / raw)
  To: Jeffrey Law; +Cc: GCC Patches, Uros Bizjak, Richard Biener, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 3362 bytes --]

On Thu, Jun 11, 2020 at 8:24 PM Jeff Law <law@redhat.com> wrote:
>
> On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote:
> > Currently patchable area is at the wrong place.  It is placed immediately
> > after function label, before both .cfi_startproc and ENDBR.  This patch
> > adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> > changes ENDBR insertion pass to also insert patchable area instruction.
> > TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> > patchable area before .cfi_startproc and ENDBR.
> >
> > OK for master?
> >
> > Thanks.
> >
> > H.J.
> > ---
> > gcc/
> >
> >       PR target/93492
> >       * config/i386/i386-features.c (rest_of_insert_endbranch):
> >       Renamed to ...
> >       (rest_of_insert_endbr_and_patchable_area): Change return type
> >       to void. Add need_endbr and patchable_area_size arguments.
> >       Don't call timevar_push nor timevar_pop.  Replace
> >       endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
> >       UNSPECV_PATCHABLE_AREA for patchable area.
> >       (pass_data_insert_endbranch): Renamed to ...
> >       (pass_data_insert_endbr_and_patchable_area): This.  Change
> >       pass name to endbr_and_patchable_area.
> >       (pass_insert_endbranch): Renamed to ...
> >       (pass_insert_endbr_and_patchable_area): This.  Add need_endbr
> >       and patchable_area_size;.
> >       (pass_insert_endbr_and_patchable_area::gate): Set and check
> >       need_endbr and patchable_area_size.
> >       (pass_insert_endbr_and_patchable_area::execute): Call
> >       timevar_push and timevar_pop.  Pass need_endbr and
> >       patchable_area_size to rest_of_insert_endbr_and_patchable_area.
> >       (make_pass_insert_endbranch): Renamed to ...
> >       (make_pass_insert_endbr_and_patchable_area): This.
> >       * config/i386/i386-passes.def: Replace pass_insert_endbranch
> >       with pass_insert_endbr_and_patchable_area.
> >       * config/i386/i386-protos.h (ix86_output_patchable_area): New.
> >       (make_pass_insert_endbranch): Renamed to ...
> >       (make_pass_insert_endbr_and_patchable_area): This.
> >       * config/i386/i386.c (ix86_asm_output_function_label): Set
> >       function_label_emitted to true.
> >       (ix86_print_patchable_function_entry): New function.
> >       (ix86_output_patchable_area): Likewise.
> >       (x86_function_profiler): Replace endbr_queued_at_entrance with
> >       insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
> >       Call ix86_output_patchable_area to generate patchable area if
> >       needed.
> >       (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
> >       * i386.h (queued_insn_type): New.
> >       (machine_function): Add function_label_emitted.  Replace
> >       endbr_queued_at_entrance with insn_queued_at_entrance.
> >       * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
> >       (patchable_area): New.
> >
> > gcc/testsuite/
> >
> >       PR target/93492
> >       * gcc.target/i386/pr93492-1.c: New test.
> >       * gcc.target/i386/pr93492-2.c: Likewise.
> >       * gcc.target/i386/pr93492-3.c: Likewise.
> >       * gcc.target/i386/pr93492-4.c: Likewise.
> >       * gcc.target/i386/pr93492-5.c: Likewise.
> OK
> jeff
> >

My patch triggered a latent x86 backend bug.   This patch fixes
it.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Linux-i386-Remove-SUBTARGET_FRAME_POINTER_REQUIRED.patch --]
[-- Type: text/x-patch, Size: 2372 bytes --]

From 0a5ebc4daad8533271406a4713059c19ebc76f56 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 12 Jun 2020 05:44:59 -0700
Subject: [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED

config/i386/gnu-user.h has

 #define SUBTARGET_FRAME_POINTER_REQUIRED crtl->profile

ix86_frame_pointer_required() has

  /* Several x86 os'es need a frame pointer for other reasons,
     usually pertaining to setjmp.  */
  if (SUBTARGET_FRAME_POINTER_REQUIRED)
    return true;
...

  if (crtl->profile && !flag_fentry)
    return true;

A frame pointer is needed only for -pg, not for -mfentry -pg.  Remove
SUBTARGET_FRAME_POINTER_REQUIRED from gnu-user.h to make i386 GCC behave
the same as x86-64 GCC.  This fixes

FAIL: gcc.target/i386/pr93492-3.c scan-assembler \t.cfi_startproc\n\tendbr(32|64)\n.*.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n
FAIL: gcc.target/i386/pr93492-5.c scan-assembler \t.cfi_startproc\n.*.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n

on Linux/i386.

	PR target/95655
	* config/i386/gnu-user.h (SUBTARGET_FRAME_POINTER_REQUIRED):
	Removed.
	* config/i386/i386.c (ix86_frame_pointer_required): Update
	comments.
---
 gcc/config/i386/gnu-user.h | 6 ------
 gcc/config/i386/i386.c     | 2 ++
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/gcc/config/i386/gnu-user.h b/gcc/config/i386/gnu-user.h
index ae4aa844f02..6ec5a114270 100644
--- a/gcc/config/i386/gnu-user.h
+++ b/gcc/config/i386/gnu-user.h
@@ -39,12 +39,6 @@ along with GCC; see the file COPYING3.  If not see
 #undef MCOUNT_NAME
 #define MCOUNT_NAME "mcount"
 
-/* The GLIBC version of mcount for the x86 assumes that there is a
-   frame, so we cannot allow profiling without a frame pointer.  */
-
-#undef SUBTARGET_FRAME_POINTER_REQUIRED
-#define SUBTARGET_FRAME_POINTER_REQUIRED crtl->profile
-
 #undef SIZE_TYPE
 #define SIZE_TYPE "unsigned int"
  
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3b776c08a22..b84c6133ed0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -5267,6 +5267,8 @@ ix86_frame_pointer_required (void)
 	  || ix86_current_function_calls_tls_descriptor))
     return true;
 
+  /* Several versions of mcount for the x86 assumes that there is a
+     frame, so we cannot allow profiling without a frame pointer.  */
   if (crtl->profile && !flag_fentry)
     return true;
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PING^2: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-06-09 16:34   ` PING^2: " H.J. Lu
@ 2020-06-16 14:17     ` Jakub Jelinek
  2020-06-16 14:24       ` H.J. Lu
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2020-06-16 14:17 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches, Jan Hubicka, Richard Biener

On Tue, Jun 09, 2020 at 09:34:01AM -0700, H.J. Lu via Gcc-patches wrote:
> > >         * gcc.target/i386/pr93492-3.c: Likewise.
> > >         * gcc.target/i386/pr93492-5.c: Likewise.

These tests FAIL on i686-linux.
E.g. in the first one I see
        .file   "pr93492-3.c"
        .text
        .globl  f10_endbr
        .type   f10_endbr, @function
f10_endbr:
.LFB0:
        .cfi_startproc
        endbr32
        .section        __patchable_function_entries,"aw",@progbits
        .align 4
        .long   .LPFE1
        .text
.LPFE1:
        nop
1:      call    __fentry__
        pushl   %ebp
        .cfi_def_cfa_offset 8
        .cfi_offset 5, -8
        movl    %esp, %ebp
        .cfi_def_cfa_register 5
        popl    %ebp
        .cfi_restore 5
        .cfi_def_cfa 4, 4
        ret
        .cfi_endproc
.LFE0:
        .size   f10_endbr, .-f10_endbr
so it doesn't match the scan regexp, because
	call	__fentry__
is not immediately followed by
	ret
As -pg is incompatible with -fomit-frame-pointer, I don't see anything wrong
on that.

Another thing in the test is that I don't think you can rely on
.cfi_startproc actually being printed, you should add an effective target
that will either check __GCC_HAVE_DWARF2_CFI_ASM macro definition, or check
for presence of .cfi_startproc on some trivial function compiled with
-fasynchronous-unwind-tables.

	Jakub


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PING^2: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-06-16 14:17     ` Jakub Jelinek
@ 2020-06-16 14:24       ` H.J. Lu
  0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2020-06-16 14:24 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches, Jan Hubicka, Richard Biener

On Tue, Jun 16, 2020 at 7:17 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Jun 09, 2020 at 09:34:01AM -0700, H.J. Lu via Gcc-patches wrote:
> > > >         * gcc.target/i386/pr93492-3.c: Likewise.
> > > >         * gcc.target/i386/pr93492-5.c: Likewise.
>
> These tests FAIL on i686-linux.
> E.g. in the first one I see
>         .file   "pr93492-3.c"
>         .text
>         .globl  f10_endbr
>         .type   f10_endbr, @function
> f10_endbr:
> .LFB0:
>         .cfi_startproc
>         endbr32
>         .section        __patchable_function_entries,"aw",@progbits
>         .align 4
>         .long   .LPFE1
>         .text
> .LPFE1:
>         nop
> 1:      call    __fentry__
>         pushl   %ebp
>         .cfi_def_cfa_offset 8
>         .cfi_offset 5, -8
>         movl    %esp, %ebp
>         .cfi_def_cfa_register 5
>         popl    %ebp
>         .cfi_restore 5
>         .cfi_def_cfa 4, 4
>         ret
>         .cfi_endproc
> .LFE0:
>         .size   f10_endbr, .-f10_endbr
> so it doesn't match the scan regexp, because
>         call    __fentry__
> is not immediately followed by
>         ret
> As -pg is incompatible with -fomit-frame-pointer, I don't see anything wrong
> on that.

Can you take a look at

https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547992.html

It should fix it.

> Another thing in the test is that I don't think you can rely on
> .cfi_startproc actually being printed, you should add an effective target
> that will either check __GCC_HAVE_DWARF2_CFI_ASM macro definition, or check
> for presence of .cfi_startproc on some trivial function compiled with
> -fasynchronous-unwind-tables.
>

-mfentry and -fpatchable-function-entry= don't work on all targets.
Should I limit these tests to Linux?

-- 
H.J.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED
  2020-06-12 12:56   ` [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED H.J. Lu
@ 2020-06-26 22:11     ` Jeff Law
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Law @ 2020-06-26 22:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak, Richard Biener, Jakub Jelinek

On Fri, 2020-06-12 at 05:56 -0700, H.J. Lu wrote:
> On Thu, Jun 11, 2020 at 8:24 PM Jeff Law <law@redhat.com> wrote:
> > On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote:
> > > Currently patchable area is at the wrong place.  It is placed immediately
> > > after function label, before both .cfi_startproc and ENDBR.  This patch
> > > adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> > > changes ENDBR insertion pass to also insert patchable area instruction.
> > > TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> > > patchable area before .cfi_startproc and ENDBR.
> > > 
> > > OK for master?
> > > 
> > > Thanks.
> > > 
> > > H.J.
> > > ---
> > > gcc/
> > > 
> > >       PR target/93492
> > >       * config/i386/i386-features.c (rest_of_insert_endbranch):
> > >       Renamed to ...
> > >       (rest_of_insert_endbr_and_patchable_area): Change return type
> > >       to void. Add need_endbr and patchable_area_size arguments.
> > >       Don't call timevar_push nor timevar_pop.  Replace
> > >       endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
> > >       UNSPECV_PATCHABLE_AREA for patchable area.
> > >       (pass_data_insert_endbranch): Renamed to ...
> > >       (pass_data_insert_endbr_and_patchable_area): This.  Change
> > >       pass name to endbr_and_patchable_area.
> > >       (pass_insert_endbranch): Renamed to ...
> > >       (pass_insert_endbr_and_patchable_area): This.  Add need_endbr
> > >       and patchable_area_size;.
> > >       (pass_insert_endbr_and_patchable_area::gate): Set and check
> > >       need_endbr and patchable_area_size.
> > >       (pass_insert_endbr_and_patchable_area::execute): Call
> > >       timevar_push and timevar_pop.  Pass need_endbr and
> > >       patchable_area_size to rest_of_insert_endbr_and_patchable_area.
> > >       (make_pass_insert_endbranch): Renamed to ...
> > >       (make_pass_insert_endbr_and_patchable_area): This.
> > >       * config/i386/i386-passes.def: Replace pass_insert_endbranch
> > >       with pass_insert_endbr_and_patchable_area.
> > >       * config/i386/i386-protos.h (ix86_output_patchable_area): New.
> > >       (make_pass_insert_endbranch): Renamed to ...
> > >       (make_pass_insert_endbr_and_patchable_area): This.
> > >       * config/i386/i386.c (ix86_asm_output_function_label): Set
> > >       function_label_emitted to true.
> > >       (ix86_print_patchable_function_entry): New function.
> > >       (ix86_output_patchable_area): Likewise.
> > >       (x86_function_profiler): Replace endbr_queued_at_entrance with
> > >       insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
> > >       Call ix86_output_patchable_area to generate patchable area if
> > >       needed.
> > >       (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
> > >       * i386.h (queued_insn_type): New.
> > >       (machine_function): Add function_label_emitted.  Replace
> > >       endbr_queued_at_entrance with insn_queued_at_entrance.
> > >       * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
> > >       (patchable_area): New.
> > > 
> > > gcc/testsuite/
> > > 
> > >       PR target/93492
> > >       * gcc.target/i386/pr93492-1.c: New test.
> > >       * gcc.target/i386/pr93492-2.c: Likewise.
> > >       * gcc.target/i386/pr93492-3.c: Likewise.
> > >       * gcc.target/i386/pr93492-4.c: Likewise.
> > >       * gcc.target/i386/pr93492-5.c: Likewise.
> > OK
> > jeff
> 
> My patch triggered a latent x86 backend bug.   This patch fixes
> it.  OK for master?
OK
jeff


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
  2020-02-04  2:11   ` H.J. Lu
@ 2020-02-04 14:54     ` H.J. Lu
  0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2020-02-04 14:54 UTC (permalink / raw)
  To: GCC Patches, Uros Bizjak

On Mon, Feb 03, 2020 at 06:10:49PM -0800, H.J. Lu wrote:
> On Mon, Feb 3, 2020 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
> > > ENDBR are emitted before the patch area.  When -mfentry -pg is also used
> > > together, there should be no ENDBR before "call __fentry__".
> > >
> > > OK for master if there is no regression?
> > >
> > > Thanks.
> > >
> > > H.J.
> > > --
> > > gcc/
> > >
> > > PR target/93492
> > > * config/i386/i386.c (ix86_asm_output_function_label): Set
> > > function_label_emitted to true.
> > > (ix86_print_patchable_function_entry): New function.
> > >
> > > gcc/testsuite/
> > >
> > > PR target/93492
> > > * gcc.target/i386/pr93492-1.c: New test.
> > > * gcc.target/i386/pr93492-2.c: Likewise.
> > > * gcc.target/i386/pr93492-3.c: Likewise.
> > >
> >
> > This version works with both .cfi_startproc and DWARF debug info.
> >
> 
> -g -fpatchable-function-entry=1  doesn't work together:
> 

Here is a differnt approach with UNSPECV_PATCHABLE_AREA.


H.J.
---
Currently patchable area is at the wrong place.  It is placed immediately
after function label, before both .cfi_startproc and ENDBR.  This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
changes ENDBR insertion pass to also insert a dummy patchable area.
TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to provide the
actual size for patchable area.  It places patchable area immediately
after .cfi_startproc and ENDBR.

gcc/

	PR target/93492
	* config/i386/i386-features.c (rest_of_insert_endbranch):
	Renamed to ...
	(rest_of_insert_endbr_and_patchable_area): Change return type
	to void.  Don't call timevar_push nor timevar_pop.  Replace
	endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
	UNSPECV_PATCHABLE_AREA for patchable area.
	(pass_data_insert_endbranch): Renamed to ...
	(pass_data_insert_endbr_and_patchable_area): This.  Change
	pass name to endbr_and_patchable_area.
	(pass_insert_endbranch): Renamed to ...
	(pass_insert_endbr_and_patchable_area): This.  Add need_endbr
	and need_patchable_area.
	(pass_insert_endbr_and_patchable_area::gate): Set and check
	need_endbr/need_patchable_area.
	(pass_insert_endbr_and_patchable_area::execute): Call
	timevar_push and timevar_pop.  Pass need_endbr amd
	need_patchable_area to rest_of_insert_endbr_and_patchable_area.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386-passes.def: Replace pass_insert_endbranch
	with pass_insert_endbr_and_patchable_area.
	* config/i386/i386-protos.h (ix86_output_patchable_area): New.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386.c (ix86_asm_output_function_label): Set
	function_label_emitted to true.
	(ix86_print_patchable_function_entry): New function.
	(ix86_output_patchable_area): Likewise.
	(x86_function_profiler): Replace endbr_queued_at_entrance with
	insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
	Call ix86_output_patchable_area to generate patchable area.
	(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
	* i386.h (queued_insn_type): New.
	(machine_function): Add patch_area_size, record_patch_area and
	function_label_emitted.  Replace endbr_queued_at_entrance with
	insn_queued_at_entrance.
	* config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
	(patchable_area): New.

gcc/testsuite/

	PR target/93492
	* gcc.target/i386/pr93492-1.c: New test.
	* gcc.target/i386/pr93492-2.c: Likewise.
	* gcc.target/i386/pr93492-3.c: Likewise.
	* gcc.target/i386/pr93492-4.c: Likewise.
	* gcc.target/i386/pr93492-5.c: Likewise.
---
 gcc/config/i386/i386-features.c           | 139 ++++++++++++++--------
 gcc/config/i386/i386-passes.def           |   2 +-
 gcc/config/i386/i386-protos.h             |   5 +-
 gcc/config/i386/i386.c                    |  90 +++++++++++++-
 gcc/config/i386/i386.h                    |  20 +++-
 gcc/config/i386/i386.md                   |  14 +++
 gcc/testsuite/gcc.target/i386/pr93492-1.c |  73 ++++++++++++
 gcc/testsuite/gcc.target/i386/pr93492-2.c |  12 ++
 gcc/testsuite/gcc.target/i386/pr93492-3.c |  13 ++
 gcc/testsuite/gcc.target/i386/pr93492-4.c |  11 ++
 gcc/testsuite/gcc.target/i386/pr93492-5.c |  12 ++
 11 files changed, 337 insertions(+), 54 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-5.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index b49e6f8d408..479b0d8fdd4 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1937,43 +1937,78 @@ make_pass_stv (gcc::context *ctxt)
   return new pass_stv (ctxt);
 }
 
-/* Inserting ENDBRANCH instructions.  */
+/* Inserting ENDBR and pseudo patchable-area instructions.  */
 
-static unsigned int
-rest_of_insert_endbranch (void)
+static void
+rest_of_insert_endbr_and_patchable_area (bool need_endbr,
+					 bool need_patchable_area)
 {
-  timevar_push (TV_MACH_DEP);
-
-  rtx cet_eb;
-  rtx_insn *insn;
+  rtx endbr;
+  rtx_insn *endbr_insn = NULL, *insn;
   basic_block bb;
 
-  /* Currently emit EB if it's a tracking function, i.e. 'nocf_check' is
-     absent among function attributes.  Later an optimization will be
-     introduced to make analysis if an address of a static function is
-     taken.  A static function whose address is not taken will get a
-     nocf_check attribute.  This will allow to reduce the number of EB.  */
+  if (need_endbr)
+    {
+      /* Currently emit EB if it's a tracking function, i.e. 'nocf_check'
+	 is absent among function attributes.  Later an optimization will
+	 be introduced to make analysis if an address of a static function
+	 is taken.  A static function whose address is not taken will get
+	 a nocf_check attribute.  This will allow to reduce the number of
+	 EB.  */
+      if (!lookup_attribute ("nocf_check",
+			     TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+	  && (!flag_manual_endbr
+	      || lookup_attribute ("cf_check",
+				   DECL_ATTRIBUTES (cfun->decl)))
+	  && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+	{
+	  if (crtl->profile && flag_fentry)
+	    {
+	      /* Queue ENDBR insertion to x86_function_profiler.
+		 NB: Any patchable-area insn will be inserted after
+		 ENDBR.  */
+	      cfun->machine->insn_queued_at_entrance = TYPE_ENDBR;
+	    }
+	  else
+	    {
+	      endbr = gen_nop_endbr ();
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      rtx_insn *insn = BB_HEAD (bb);
+	      endbr_insn = emit_insn_before (endbr, insn);
+	    }
+	}
+    }
 
-  if (!lookup_attribute ("nocf_check",
-			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
-      && (!flag_manual_endbr
-	  || lookup_attribute ("cf_check",
-			       DECL_ATTRIBUTES (cfun->decl)))
-      && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+  if (need_patchable_area)
     {
-      /* Queue ENDBR insertion to x86_function_profiler.  */
       if (crtl->profile && flag_fentry)
-	cfun->machine->endbr_queued_at_entrance = true;
+	{
+	  /* Queue patchable-area insertion to x86_function_profiler.
+	     NB: If there is a queued ENDBR, x86_function_profiler
+	     will also handle patchable-area.  */
+	  if (!cfun->machine->insn_queued_at_entrance)
+	    cfun->machine->insn_queued_at_entrance = TYPE_PATCHABLE_AREA;
+	}
       else
 	{
-	  cet_eb = gen_nop_endbr ();
-
-	  bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
-	  insn = BB_HEAD (bb);
-	  emit_insn_before (cet_eb, insn);
+	  /* ix86_print_patchable_function_entry will provide actual
+	     size.  */
+	  rtx patchable_area = gen_patchable_area (GEN_INT (0),
+						   GEN_INT (0));
+	  if (endbr_insn)
+	    emit_insn_after (patchable_area, endbr_insn);
+	  else
+	    {
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      insn = BB_HEAD (bb);
+	      emit_insn_before (patchable_area, insn);
+	    }
 	}
     }
 
+  if (!need_endbr)
+    return;
+
   bb = 0;
   FOR_EACH_BB_FN (bb, cfun)
     {
@@ -1982,7 +2017,6 @@ rest_of_insert_endbranch (void)
 	{
 	  if (CALL_P (insn))
 	    {
-	      bool need_endbr;
 	      need_endbr = find_reg_note (insn, REG_SETJMP, NULL) != NULL;
 	      if (!need_endbr && !SIBLING_CALL_P (insn))
 		{
@@ -2013,8 +2047,8 @@ rest_of_insert_endbranch (void)
 	      /* Generate ENDBRANCH after CALL, which can return more than
 		 twice, setjmp-like functions.  */
 
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after_setloc (cet_eb, insn, INSN_LOCATION (insn));
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after_setloc (endbr, insn, INSN_LOCATION (insn));
 	      continue;
 	    }
 
@@ -2039,36 +2073,33 @@ rest_of_insert_endbranch (void)
 
 	      FOR_EACH_EDGE (e, ei, bb->succs)
 		{
-		  rtx_insn *insn;
-
 		  dest_blk = e->dest;
 		  insn = BB_HEAD (dest_blk);
 		  gcc_assert (LABEL_P (insn));
-		  cet_eb = gen_nop_endbr ();
-		  emit_insn_after (cet_eb, insn);
+		  endbr = gen_nop_endbr ();
+		  emit_insn_after (endbr, insn);
 		}
 	      continue;
 	    }
 
 	  if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
 	    {
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after (cet_eb, insn);
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after (endbr, insn);
 	      continue;
 	    }
 	}
     }
 
-  timevar_pop (TV_MACH_DEP);
-  return 0;
+  return;
 }
 
 namespace {
 
-const pass_data pass_data_insert_endbranch =
+const pass_data pass_data_insert_endbr_and_patchable_area =
 {
   RTL_PASS, /* type.  */
-  "cet", /* name.  */
+  "endbr_and_patchable_area", /* name.  */
   OPTGROUP_NONE, /* optinfo_flags.  */
   TV_MACH_DEP, /* tv_id.  */
   0, /* properties_required.  */
@@ -2078,32 +2109,44 @@ const pass_data pass_data_insert_endbranch =
   0, /* todo_flags_finish.  */
 };
 
-class pass_insert_endbranch : public rtl_opt_pass
+class pass_insert_endbr_and_patchable_area : public rtl_opt_pass
 {
 public:
-  pass_insert_endbranch (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_insert_endbranch, ctxt)
+  pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
+    : rtl_opt_pass (pass_data_insert_endbr_and_patchable_area, ctxt)
   {}
 
   /* opt_pass methods: */
-  virtual bool gate (function *)
-    {
-      return ((flag_cf_protection & CF_BRANCH));
+  virtual bool gate (function *fun)
+    {
+      need_endbr = (flag_cf_protection & CF_BRANCH) != 0;
+      need_patchable_area
+	= (function_entry_patch_area_size
+	   || lookup_attribute ("patchable_function_entry",
+				DECL_ATTRIBUTES (fun->decl)));
+      return need_endbr || need_patchable_area;
     }
 
   virtual unsigned int execute (function *)
     {
-      return rest_of_insert_endbranch ();
+      timevar_push (TV_MACH_DEP);
+      rest_of_insert_endbr_and_patchable_area (need_endbr,
+					       need_patchable_area);
+      timevar_pop (TV_MACH_DEP);
+      return 0;
     }
 
-}; // class pass_insert_endbranch
+private:
+  bool need_endbr;
+  bool need_patchable_area;
+}; // class pass_insert_endbr_and_patchable_area
 
 } // anon namespace
 
 rtl_opt_pass *
-make_pass_insert_endbranch (gcc::context *ctxt)
+make_pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
 {
-  return new pass_insert_endbranch (ctxt);
+  return new pass_insert_endbr_and_patchable_area (ctxt);
 }
 
 /* At entry of the nearest common dominator for basic blocks with
diff --git a/gcc/config/i386/i386-passes.def b/gcc/config/i386/i386-passes.def
index 41386a13d88..d83c7b956b1 100644
--- a/gcc/config/i386/i386-passes.def
+++ b/gcc/config/i386/i386-passes.def
@@ -30,6 +30,6 @@ along with GCC; see the file COPYING3.  If not see
      CONSTM1_RTX generated by the STV pass can be CSEed.  */
   INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
 
-  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbranch);
+  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbr_and_patchable_area);
 
   INSERT_PASS_AFTER (pass_combine, 1, pass_remove_partial_avx_dependency);
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 266381ca5a6..cbcc08ebfe8 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -87,6 +87,8 @@ extern const char *output_fp_compare (rtx_insn *, rtx*, bool, bool);
 extern const char *output_adjust_stack_and_probe (rtx);
 extern const char *output_probe_stack_range (rtx, rtx);
 
+extern void ix86_output_patchable_area (unsigned int, bool);
+
 extern void ix86_expand_clear (rtx);
 extern void ix86_expand_move (machine_mode, rtx[]);
 extern void ix86_expand_vector_move (machine_mode, rtx[]);
@@ -376,6 +378,7 @@ class rtl_opt_pass;
 
 extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
 extern rtl_opt_pass *make_pass_stv (gcc::context *);
-extern rtl_opt_pass *make_pass_insert_endbranch (gcc::context *);
+extern rtl_opt_pass *make_pass_insert_endbr_and_patchable_area
+  (gcc::context *);
 extern rtl_opt_pass *make_pass_remove_partial_avx_dependency
   (gcc::context *);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffda3e8fd21..071bce8733f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1563,6 +1563,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
 {
   bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
 
+  if (cfun)
+    cfun->machine->function_label_emitted = true;
+
   if (is_ms_hook)
     {
       int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9118,6 +9121,80 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
     }
 }
 
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY.  */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+				     unsigned HOST_WIDE_INT patch_area_size,
+				     bool record_p)
+{
+  if (cfun->machine->function_label_emitted)
+    {
+      /* The insert_endbr_and_patchable_area pass inserted a dummy
+	 UNSPECV_PATCHABLE_AREA with 0 patchable area size.  If the
+	 patchable area is placed after the function label, we replace
+	 0 patchable area size with the real one.  Otherwise, the
+	 dummy UNSPECV_PATCHABLE_AREA will be ignored.  */
+      if (cfun->machine->insn_queued_at_entrance)
+	{
+	  /* Record the patchable area.  Both ENDBR and patchable area
+	     will be inserted by x86_function_profiler later.  */
+	  cfun->machine->patch_area_size = patch_area_size;
+	  cfun->machine->record_patch_area = record_p;
+	  return;
+	}
+
+      /* We can have
+
+	 UNSPECV_NOP_ENDBR
+	 UNSPECV_PATCHABLE_AREA
+
+	 or just
+
+	 UNSPECV_PATCHABLE_AREA
+       */
+      rtx_insn *patchable_insn;
+      rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+      if (insn
+	  && INSN_P (insn)
+	  && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
+	  && XINT (PATTERN (insn), 1) == UNSPECV_NOP_ENDBR)
+	patchable_insn = next_real_nondebug_insn (insn);
+      else
+	patchable_insn = insn;
+
+      if (patchable_insn && INSN_P (patchable_insn))
+	{
+	  /* Replace the dummy patchable area size with the real one.  */
+	  rtx pattern = PATTERN (patchable_insn);
+	  if (GET_CODE (pattern) == UNSPEC_VOLATILE
+	      && XINT (pattern, 1) == UNSPECV_PATCHABLE_AREA)
+	    {
+	      XVECEXP (pattern, 0, 0) = GEN_INT (patch_area_size);
+	      XVECEXP (pattern, 0, 1) = GEN_INT (record_p);
+	    }
+	  return;
+	}
+
+      gcc_unreachable ();
+    }
+
+  default_print_patchable_function_entry (file, patch_area_size,
+					  record_p);
+}
+
+/* Output patchable area.  */
+
+void
+ix86_output_patchable_area (unsigned int patch_area_size,
+			    bool record_p)
+{
+  if (patch_area_size)
+    default_print_patchable_function_entry (asm_out_file,
+					    patch_area_size,
+					    record_p);
+}
+
 /* Return a scratch register to use in the split stack prologue.  The
    split stack prologue is used for -fsplit-stack.  It is the first
    instructions in the function, even before the regular prologue.
@@ -20151,8 +20228,13 @@ current_fentry_section (const char **name)
 void
 x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
 {
-  if (cfun->machine->endbr_queued_at_entrance)
-    fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+  if (cfun->machine->insn_queued_at_entrance)
+    {
+      if (cfun->machine->insn_queued_at_entrance == TYPE_ENDBR)
+	fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+      ix86_output_patchable_area (cfun->machine->patch_area_size,
+				  cfun->machine->record_patch_area);
+    }
 
   const char *mcount_name = MCOUNT_NAME;
 
@@ -22744,6 +22826,10 @@ ix86_run_selftests (void)
 #undef TARGET_ASM_FUNCTION_EPILOGUE
 #define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  ix86_print_patchable_function_entry
+
 #undef TARGET_ENCODE_SECTION_INFO
 #ifndef SUBTARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 943e9a5c783..7c8f0cd4c70 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2751,6 +2751,13 @@ enum function_type
   TYPE_EXCEPTION
 };
 
+enum queued_insn_type
+{
+  TYPE_NONE = 0,
+  TYPE_ENDBR,
+  TYPE_PATCHABLE_AREA
+};
+
 struct GTY(()) machine_function {
   struct stack_local_entry *stack_locals;
   int varargs_gpr_size;
@@ -2767,6 +2774,12 @@ struct GTY(()) machine_function {
      structure.  */
   rtx split_stack_varargs_pointer;
 
+  /* The size of the patchable area at function entry.  */
+  unsigned int patch_area_size;
+
+  /* If true, record patchable area at function entry.  */
+  BOOL_BITFIELD record_patch_area : 1;
+
   /* This value is used for amd64 targets and specifies the current abi
      to be used. MS_ABI means ms abi. Otherwise SYSV_ABI means sysv abi.  */
   ENUM_BITFIELD(calling_abi) call_abi : 8;
@@ -2841,8 +2854,11 @@ struct GTY(()) machine_function {
   /* Nonzero if the function places outgoing arguments on stack.  */
   BOOL_BITFIELD outgoing_args_on_stack : 1;
 
-  /* If true, ENDBR is queued at function entrance.  */
-  BOOL_BITFIELD endbr_queued_at_entrance : 1;
+  /* If true, ENDBR or patchable area is queued at function entrance.  */
+  ENUM_BITFIELD(queued_insn_type) insn_queued_at_entrance : 2;
+
+  /* If true, the function label has been emitted.  */
+  BOOL_BITFIELD function_label_emitted : 1;
 
   /* True if the function needs a stack frame.  */
   BOOL_BITFIELD stack_frame_required : 1;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 46b442dae51..75bf8fd2348 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -300,6 +300,9 @@ (define_c_enum "unspecv" [
   ;; For ENQCMD and ENQCMDS support
   UNSPECV_ENQCMD
   UNSPECV_ENQCMDS
+
+  ;; For patchable area support
+  UNSPECV_PATCHABLE_AREA
 ])
 
 ;; Constants to represent rounding modes in the ROUND instruction
@@ -21280,6 +21283,17 @@ (define_insn "speculation_barrier"
   [(set_attr "type" "other")
    (set_attr "length" "3")])
 
+(define_insn "patchable_area"
+  [(unspec_volatile [(match_operand 0 "const_int_operand")
+		     (match_operand 1 "const_int_operand")]
+		    UNSPECV_PATCHABLE_AREA)]
+  ""
+{
+  ix86_output_patchable_area (INTVAL (operands[0]),
+			      INTVAL (operands[1]) != 0);
+  return "";
+})
+
 (include "mmx.md")
 (include "sse.md")
 (include "sync.md")
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-1.c b/gcc/testsuite/gcc.target/i386/pr93492-1.c
new file mode 100644
index 00000000000..f978d2e5220
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-1.c
@@ -0,0 +1,73 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* Note: this test only checks the instructions in the function bodies,
+   not the placement of the patch label or nops before the function.  */
+
+/*
+**f10_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 0)))
+f10_none (void)
+{
+}
+
+/*
+**f10_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/*
+**f11_none:
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 1)))
+f11_none (void)
+{
+}
+
+/*
+**f11_endbr:
+**	endbr(32|64)
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 1)))
+f11_endbr (void)
+{
+}
+
+/*
+**f21_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (2, 1)))
+f21_none (void)
+{
+}
+
+/*
+**f21_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (2, 1)))
+f21_endbr (void)
+{
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-2.c b/gcc/testsuite/gcc.target/i386/pr93492-2.c
new file mode 100644
index 00000000000..ec26d4cc367
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-2.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-3.c b/gcc/testsuite/gcc.target/i386/pr93492-3.c
new file mode 100644
index 00000000000..1f03c627120
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-3.c
@@ -0,0 +1,13 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-4.c b/gcc/testsuite/gcc.target/i386/pr93492-4.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-4.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-5.c b/gcc/testsuite/gcc.target/i386/pr93492-5.c
new file mode 100644
index 00000000000..d04077c6007
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-5.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
-- 
2.24.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-26 22:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-02 11:55 [PATCH] x86: Add UNSPECV_PATCHABLE_AREA H.J. Lu
2020-05-22 11:22 ` PING: " H.J. Lu
2020-06-09 16:34   ` PING^2: " H.J. Lu
2020-06-16 14:17     ` Jakub Jelinek
2020-06-16 14:24       ` H.J. Lu
2020-06-12  3:24 ` Jeff Law
2020-06-12 12:56   ` [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED H.J. Lu
2020-06-26 22:11     ` Jeff Law
  -- strict thread matches above, loose matches on Subject: below --
2020-02-03 18:36 [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY H.J. Lu
2020-02-04  0:02 ` H.J. Lu
2020-02-04  2:11   ` H.J. Lu
2020-02-04 14:54     ` [PATCH] x86: Add UNSPECV_PATCHABLE_AREA H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).