[PATCH v10] add -fpatchable-function-entry=N,M option

[PATCH v10] add -fpatchable-function-entry=N,M option

[Torsten Duwe]

Changes since v9:

* Do not store (declare static) the nop pattern template string.
  In the future, it might depend on the particular function
  being emitted. Fetch it freshly each time instead.

* On platforms without named sections, simply omit the recording
  of the nop locations. Run-time instrumentation can still fiddle
  it out, if desired. Document this behaviour in a half sentence.

* Move the hook documentation to where it belongs. Texi file (re-)
  generation should work cleanly now.

* Documentation clarified as requested.

	Torsten

---
gcc/c-family/ChangeLog
2017-07-04  Torsten Duwe  <duwe@suse.de>

	* c-attribs.c (c_common_attribute_table): Add entry for
	"patchable_function_entry".

gcc/lto/ChangeLog
2017-07-04  Torsten Duwe  <duwe@suse.de>

	* lto-lang.c (lto_attribute_table): Add entry for
	"patchable_function_entry".

gcc/ChangeLog
2017-07-04  Torsten Duwe  <duwe@suse.de>

	* common.opt: Introduce -fpatchable-function-entry
	command line option, and its variables function_entry_patch_area_size
	and function_entry_patch_area_start.
	* opts.c (common_handle_option): Add -fpatchable_function_entry_ case,
	including a two-value parser.
	* target.def (print_patchable_function_entry): New target hook.
	* targhooks.h (default_print_patchable_function_entry): New function.
	* targhooks.c (default_print_patchable_function_entry): Likewise.
	* toplev.c (process_options): Switch off IPA-RA if
	patchable function entries are being generated.
	* varasm.c (assemble_start_function): Look at the
	patchable-function-entry command line switch and current
	function attributes and maybe generate NOP instructions by
	calling the print_patchable_function_entry hook.
	* doc/extend.texi: Document patchable_function_entry attribute.
	* doc/invoke.texi: Document -fpatchable_function_entry
	command line option.
	* doc/tm.texi.in (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY):
	New target hook.
	* doc/tm.texi: Likewise.

gcc/testsuite/ChangeLog
2017-07-04  Torsten Duwe  <duwe@suse.de>

	* c-c++-common/patchable_function_entry-default.c: New test.
	* c-c++-common/patchable_function_entry-decl.c: Likewise.
	* c-c++-common/patchable_function_entry-definition.c: Likewise.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 626ffa1cde7..ecb00c1d5b9 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -142,6 +142,8 @@ static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool *)
 static tree handle_bnd_legacy (tree *, tree, tree, int, bool *);
 static tree handle_bnd_instrument (tree *, tree, tree, int, bool *);
 static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *);
+static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
+						       int, bool *);
 
 /* Table of machine-independent attributes common to all C-like languages.
 
@@ -351,6 +353,9 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_bnd_instrument, false },
   { "fallthrough",	      0, 0, false, false, false,
 			      handle_fallthrough_attribute, false },
+  { "patchable_function_entry",	1, 2, true, false, false,
+			      handle_patchable_function_entry_attribute,
+			      false },
   { NULL,                     0, 0, false, false, false, NULL, false }
 };
 
@@ -3260,3 +3265,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
   *no_add_attrs = true;
   return NULL_TREE;
 }
+
+static tree
+handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *)
+{
+  /* Nothing to be done here.  */
+  return NULL_TREE;
+}
diff --git a/gcc/common.opt b/gcc/common.opt
index e81165c488b..78cfa568a95 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -163,6 +163,13 @@ bool flag_stack_usage_info = false
 Variable
 int flag_debug_asm
 
+; How many NOP insns to place at each function entry by default
+Variable
+HOST_WIDE_INT function_entry_patch_area_size
+
+; And how far the real asm entry point is into this area
+Variable
+HOST_WIDE_INT function_entry_patch_area_start
 
 ; Balance between GNAT encodings and standard DWARF to emit.
 Variable
@@ -2030,6 +2037,10 @@ fprofile-reorder-functions
 Common Report Var(flag_profile_reorder_functions)
 Enable function reordering that improves code placement.
 
+fpatchable-function-entry=
+Common Joined Optimization
+Insert NOP instructions at each function entry.
+
 frandom-seed
 Common Var(common_deferred_options) Defer
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 03ba8fc436c..a4c3c98b9f5 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3105,6 +3105,27 @@ that affect more than one function.
 This attribute should be used for debugging purposes only.  It is not
 suitable in production code.
 
+@item patchable_function_entry
+@cindex @code{patchable_function_entry} function attribute
+@cindex extra NOP instructions at the function entry point
+In case the target's text segment can be made writable at run time by
+any means, padding the function entry with a number of NOPs can be
+used to provide a universal tool for instrumentation.
+
+The @code{patchable_function_entry} function attribute can be used to
+change the number of NOPs to any desired value.  The two-value syntax
+is the same as for the command-line switch
+@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with
+the function entry point before the @var{M}th NOP instruction.
+@var{M} defaults to 0 if omitted e.g. function entry point is before
+the first NOP.
+
+If patchable function entries are enabled globally using the command
+line option @option{-fpatchable-function-entry=N,M}, then all functions
+that are part of the instrumentation framework must disable
+instrumentation with the attribute @code{patchable_function_entry (0)}
+to prevent recursion.
+
 @item pure
 @cindex @code{pure} function attribute
 @cindex functions that have no side effects
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 04cecf94405..1b8a4555b33 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11520,6 +11520,34 @@ of the function name, it is considered to be a match.  For C99 and C++
 extended identifiers, the function name must be given in UTF-8, not
 using universal character names.
 
+@item -fpatchable-function-entry=@var{N}[,@var{M}]
+@opindex fpatchable-function-entry
+Generate @var{N} NOPs right at the beginning
+of each function, with the function entry point before the @var{M}th NOP.
+If @var{M} is omitted, it defaults to @code{0} so the
+function entry points to the address just at the first NOP.
+The NOP instructions reserve extra space which can be used to patch in
+any desired instrumentation at run time, provided that the code segment
+is writable.  The amount of space is controllable indirectly via
+the number of NOPs; the NOP instruction used corresponds to the instruction
+emitted by the internal GCC back-end interface @code{gen_nop}.  This behavior
+is target-specific and may also depend on the architecture variant and/or
+other compilation options.
+
+For run-time identification, the starting addresses of these areas,
+which correspond to their respective function entries minus @var{M},
+are additionally collected in the @code{__patchable_function_entries}
+section of the resulting binary, if the platform supports it.
+
+Note that the value of @code{__attribute__ ((patchable_function_entry
+(N,M)))} takes precedence over command-line option
+@option{-fpatchable-function-entry=N,M}.  This can be used to increase
+the area size or to remove it completely on a single function.
+If @code{N=0}, no pad location is recorded.
+
+The NOP instructions are inserted at --- and maybe before, depending on
+@var{M} --- the function entry address, even before the prologue.
+
 @end table
 
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 39302f3e883..b50e75b4bbd 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4573,6 +4573,14 @@ will select the smallest suitable mode.
 This section describes the macros that output function entry
 (@dfn{prologue}) and exit (@dfn{epilogue}) code.
 
+@deftypefn {Target Hook} void TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY (FILE *@var{file}, unsigned HOST_WIDE_INT @var{patch_area_size}, bool @var{record_p})
+Generate a patchable area at the function start, consisting of
+@var{patch_area_size} NOP instructions.  If the target supports named
+sections and if @var{record_p} is true, insert a pointer to the current
+location in the table of patchable functions.  This table will then be held
+in a special section called @code{__patchable_function_entries}.
+@end deftypefn
+
 @deftypefn {Target Hook} void TARGET_ASM_FUNCTION_PROLOGUE (FILE *@var{file}, HOST_WIDE_INT @var{size})
 If defined, a function that outputs the assembler code for entry to a
 function.  The prologue is responsible for setting up the stack frame,
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 98f2e6bce5f..6df08a2c477 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3650,6 +3650,8 @@ will select the smallest suitable mode.
 This section describes the macros that output function entry
 (@dfn{prologue}) and exit (@dfn{epilogue}) code.
 
+@hook TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+
 @hook TARGET_ASM_FUNCTION_PROLOGUE
 
 @hook TARGET_ASM_FUNCTION_END_PROLOGUE
diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c
index 58935172b2c..6e9a138fa3b 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -48,6 +48,8 @@ static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *);
 static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *);
 static tree handle_transaction_pure_attribute (tree *, tree, tree, int, bool *);
 static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *);
+static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
+						       int, bool *);
 static tree ignore_attribute (tree *, tree, tree, int, bool *);
 
 static tree handle_format_attribute (tree *, tree, tree, int, bool *);
@@ -76,6 +78,9 @@ const struct attribute_spec lto_attribute_table[] =
 			      handle_nonnull_attribute, false },
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
+  { "patchable_function_entry", 1, 2, true, false, false,
+			      handle_patchable_function_entry_attribute,
+			      false },
   { "returns_twice",          0, 0, true,  false, false,
 			      handle_returns_twice_attribute, false },
   { "sentinel",               0, 1, false, true, true,
@@ -473,6 +478,13 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
   return NULL_TREE;
 }
 
+static tree
+handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *)
+{
+  /* Nothing to be done here.  */
+  return NULL_TREE;
+}
+
 /* Ignore the given attribute.  Used when this attribute may be usefully
    overridden by the target, but is not used generically.  */
 
diff --git a/gcc/opts.c b/gcc/opts.c
index 7460c2be1b6..10c52976c90 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2196,6 +2196,33 @@ common_handle_option (struct gcc_options *opts,
         opts->x_flag_ipa_reference = false;
       break;
 
+    case OPT_fpatchable_function_entry_:
+      {
+	char *patch_area_arg = xstrdup (arg);
+	char *comma = strchr (patch_area_arg, ',');
+	if (comma)
+	  {
+	    *comma = '\0';
+	    function_entry_patch_area_size = 
+	      integral_argument (patch_area_arg);
+	    function_entry_patch_area_start =
+	      integral_argument (comma + 1);
+	  }
+	else
+	  {
+	    function_entry_patch_area_size =
+	      integral_argument (patch_area_arg);
+	    function_entry_patch_area_start = 0;
+	  }
+	if (function_entry_patch_area_size < 0
+	    || function_entry_patch_area_start < 0
+	    || function_entry_patch_area_size 
+		< function_entry_patch_area_start)
+	  error ("invalid arguments for %<-fpatchable_function_entry%>");
+	free (patch_area_arg);
+      }
+      break;
+
     case OPT_ftree_vectorize:
       if (!opts_set->x_flag_tree_loop_vectorize)
         opts->x_flag_tree_loop_vectorize = value;
diff --git a/gcc/target.def b/gcc/target.def
index bd60484c4fd..15f7549151a 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -288,6 +288,16 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
  void, (tree decl, int visibility),
  default_assemble_visibility)
 
+DEFHOOK
+(print_patchable_function_entry,
+ "Generate a patchable area at the function start, consisting of\n\
+@var{patch_area_size} NOP instructions.  If the target supports named\n\
+sections and if @var{record_p} is true, insert a pointer to the current\n\
+location in the table of patchable functions.  This table will then be held\n\
+in a special section called @code{__patchable_function_entries}.",
+ void, (FILE *file, unsigned HOST_WIDE_INT patch_area_size, bool record_p),
+ default_print_patchable_function_entry)
+
 /* Output the assembler code for entry to a function.  */
 DEFHOOK
 (function_prologue,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 889189c5d82..9ebfdedfefb 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1610,6 +1610,53 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
+/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function
+   entry.  If RECORD_P is true and the target supports named sections,
+   the location of the NOPs will be recorded in a special object section
+   called "__patchable_function_entries".  This routine may be called
+   twice per function to put NOPs before and after the function
+   entry.  */
+
+void
+default_print_patchable_function_entry (FILE *file,
+					unsigned HOST_WIDE_INT patch_area_size,
+					bool record_p)
+{
+  const char *nop_templ = 0;
+  int code_num;
+  rtx_insn *my_nop = make_insn_raw (gen_nop ());
+
+  /* We use the template alone, relying on the (currently sane) assumption
+     that the NOP template does not have variable operands.  */
+  code_num = recog_memoized (my_nop);
+  nop_templ = get_insn_template (code_num, my_nop);
+
+  if (record_p)
+    {
+#if TARGET_HAVE_NAMED_SECTIONS
+      char buf[256];
+      static int patch_area_number;
+      section *previous_section = in_section;
+
+      patch_area_number++;
+      ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number);
+
+      switch_to_section (get_section ("__patchable_function_entries",
+				      0, NULL));
+      fputs (integer_asm_op (POINTER_SIZE_UNITS, false), file);
+      assemble_name_raw (file, buf);
+      fputc ('\n', file);
+
+      switch_to_section (previous_section);
+      ASM_OUTPUT_LABEL (file, buf);
+#endif
+    }
+
+  unsigned i;
+  for (i = 0; i < patch_area_size; ++i)
+    fprintf (file, "\t%s\n", nop_templ);
+}
+
 bool
 default_profile_before_prologue (void)
 {
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 18070df7839..6206fe20823 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,9 @@ extern bool default_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT,
 						    bool);
 extern int default_compare_by_pieces_branch_ratio (machine_mode);
 
+extern void default_print_patchable_function_entry (FILE *,
+						    unsigned HOST_WIDE_INT,
+						    bool);
 extern bool default_profile_before_prologue (void);
 extern reg_class_t default_preferred_reload_class (rtx, reg_class_t);
 extern reg_class_t default_preferred_output_reload_class (rtx, reg_class_t);
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
new file mode 100644
index 00000000000..8514b10e820
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-final { scan-assembler-times "nop" 2 } } */
+
+extern int a;
+
+/* Respect overriding attributes in the declaration.  */
+int f3 (void) __attribute__((patchable_function_entry(2)));
+
+/* F3 should now get 2 NOPs.  */
+int
+__attribute__((noinline))
+f3 (void)
+{
+  return 5*a;
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c
new file mode 100644
index 00000000000..0dcf1181dde
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-final { scan-assembler-times "nop" 3 } } */
+
+extern int a;
+
+/* Nothing declared must not mean anything.  */
+int f3 (void);
+
+/* F3 should get a default-sized NOP area.  */
+int
+__attribute__((noinline))
+f3 (void)
+{
+  return 5*a;
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c
new file mode 100644
index 00000000000..a007867dcb0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-final { scan-assembler-times "nop" 1 } } */
+
+extern int a;
+
+int f3 (void);
+
+/* F3 should now get 1 NOP.  */
+int
+__attribute__((noinline))
+__attribute__((patchable_function_entry(1)))
+f3 (void)
+{
+  return 5*a;
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index e6c69a4ba93..b28f1847c83 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1630,8 +1630,10 @@ process_options (void)
     }
 
  /* Do not use IPA optimizations for register allocation if profiler is active
+    or patchable function entries are inserted for run-time instrumentation
     or port does not emit prologue and epilogue as RTL.  */
-  if (profile_flag || !targetm.have_prologue () || !targetm.have_epilogue ())
+  if (profile_flag || function_entry_patch_area_size
+      || !targetm.have_prologue () || !targetm.have_epilogue ())
     flag_ipa_ra = 0;
 
   /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error
diff --git a/gcc/varasm.c b/gcc/varasm.c
index fbaebc1b5c0..ec1640ea378 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1825,6 +1825,46 @@ assemble_start_function (tree decl, const char *fnname)
   if (DECL_PRESERVE_P (decl))
     targetm.asm_out.mark_decl_preserved (fnname);
 
+  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+  tree patchable_function_entry_attr
+    = lookup_attribute ("patchable_function_entry", DECL_ATTRIBUTES (decl));
+  if (patchable_function_entry_attr)
+    {
+      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+      if (tree_fits_uhwi_p (patchable_function_entry_value1))
+	patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+      else
+	gcc_unreachable ();
+
+      patch_area_entry = 0;
+      if (list_length (pp_val) > 1)
+	{
+	  tree patchable_function_entry_value2 =
+	    TREE_VALUE (TREE_CHAIN (pp_val));
+
+	  if (tree_fits_uhwi_p (patchable_function_entry_value2))
+	    patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+	  else
+	    gcc_unreachable ();
+	}
+    }
+
+  if (patch_area_entry > patch_area_size)
+    {
+      if (patch_area_size > 0)
+	warning (OPT_Wattributes, "Patchable function entry > size");
+      patch_area_entry = 0;
+    }
+
+  /* Emit the patching area before the entry label, if any.  */
+  if (patch_area_entry > 0)
+    targetm.asm_out.print_patchable_function_entry (asm_out_file,
+						    patch_area_entry, true);
+
   /* Do any machine/system dependent processing of the function name.  */
 #ifdef ASM_DECLARE_FUNCTION_NAME
   ASM_DECLARE_FUNCTION_NAME (asm_out_file, fnname, current_function_decl);
@@ -1833,6 +1873,12 @@ assemble_start_function (tree decl, const char *fnname)
   ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, fnname, current_function_decl);
 #endif /* ASM_DECLARE_FUNCTION_NAME */
 
+  /* And the area after the label.  Record it if we haven't done so yet.  */
+  if (patch_area_size > patch_area_entry)
+    targetm.asm_out.print_patchable_function_entry (asm_out_file,
+					     patch_area_size-patch_area_entry,
+						    patch_area_entry == 0);
+
   if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
     saw_no_split_stack = true;
 }

Re: [PATCH v10] add -fpatchable-function-entry=N,M option

[Sandra Loosemore]

On 07/05/2017 01:36 AM, Torsten Duwe wrote:
> Changes since v9:
>
> * Do not store (declare static) the nop pattern template string.
>    In the future, it might depend on the particular function
>    being emitted. Fetch it freshly each time instead.
>
> * On platforms without named sections, simply omit the recording
>    of the nop locations. Run-time instrumentation can still fiddle
>    it out, if desired. Document this behaviour in a half sentence.
>
> * Move the hook documentation to where it belongs. Texi file (re-)
>    generation should work cleanly now.
>
> * Documentation clarified as requested.
>
> 	Torsten
>
> [snip]
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 03ba8fc436c..a4c3c98b9f5 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3105,6 +3105,27 @@ that affect more than one function.
>   This attribute should be used for debugging purposes only.  It is not
>   suitable in production code.
>
> +@item patchable_function_entry
> +@cindex @code{patchable_function_entry} function attribute
> +@cindex extra NOP instructions at the function entry point
> +In case the target's text segment can be made writable at run time by
> +any means, padding the function entry with a number of NOPs can be
> +used to provide a universal tool for instrumentation.
> +
> +The @code{patchable_function_entry} function attribute can be used to
> +change the number of NOPs to any desired value.  The two-value syntax
> +is the same as for the command-line switch
> +@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with
> +the function entry point before the @var{M}th NOP instruction.
> +@var{M} defaults to 0 if omitted e.g. function entry point is before
> +the first NOP.
> +
> +If patchable function entries are enabled globally using the command
> +line option @option{-fpatchable-function-entry=N,M}, then all functions

s/command line option/command-line option

> +that are part of the instrumentation framework must disable
> +instrumentation with the attribute @code{patchable_function_entry (0)}
> +to prevent recursion.

The functions don't disable instrumentation, programmers disable the 
instrumentation on functions.  Rewrite this clause as

...then you must disable instrumentation on all functions that are part 
of the instrumentation framework with the attribute...

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 04cecf94405..1b8a4555b33 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11520,6 +11520,34 @@ of the function name, it is considered to be a match.  For C99 and C++
>   extended identifiers, the function name must be given in UTF-8, not
>   using universal character names.
>
> +@item -fpatchable-function-entry=@var{N}[,@var{M}]
> +@opindex fpatchable-function-entry
> +Generate @var{N} NOPs right at the beginning
> +of each function, with the function entry point before the @var{M}th NOP.
> +If @var{M} is omitted, it defaults to @code{0} so the
> +function entry points to the address just at the first NOP.
> +The NOP instructions reserve extra space which can be used to patch in
> +any desired instrumentation at run time, provided that the code segment
> +is writable.  The amount of space is controllable indirectly via
> +the number of NOPs; the NOP instruction used corresponds to the instruction
> +emitted by the internal GCC back-end interface @code{gen_nop}.  This behavior
> +is target-specific and may also depend on the architecture variant and/or
> +other compilation options.
> +
> +For run-time identification, the starting addresses of these areas,
> +which correspond to their respective function entries minus @var{M},
> +are additionally collected in the @code{__patchable_function_entries}
> +section of the resulting binary, if the platform supports it.
> +
> +Note that the value of @code{__attribute__ ((patchable_function_entry
> +(N,M)))} takes precedence over command-line option
> +@option{-fpatchable-function-entry=N,M}.  This can be used to increase
> +the area size or to remove it completely on a single function.
> +If @code{N=0}, no pad location is recorded.
> +
> +The NOP instructions are inserted at --- and maybe before, depending on
> +@var{M} --- the function entry address, even before the prologue.

No spaces around the em-dashes '---'.

> +DEFHOOK
> +(print_patchable_function_entry,
> + "Generate a patchable area at the function start, consisting of\n\
> +@var{patch_area_size} NOP instructions.  If the target supports named\n\
> +sections and if @var{record_p} is true, insert a pointer to the current\n\
> +location in the table of patchable functions.  This table will then be held\n\
> +in a special section called @code{__patchable_function_entries}.",

I don't understand when "then" might be.  Can you rewrite this in the 
present tense?  "This table is held..."

-Sandra

Re: [PATCH v10] add -fpatchable-function-entry=N,M option

[Richard Earnshaw (lists)]

On 05/07/17 16:38, Sandra Loosemore wrote:
> On 07/05/2017 01:36 AM, Torsten Duwe wrote:
>> Changes since v9:
>>
>> * Do not store (declare static) the nop pattern template string.
>>    In the future, it might depend on the particular function
>>    being emitted. Fetch it freshly each time instead.
>>
>> * On platforms without named sections, simply omit the recording
>>    of the nop locations. Run-time instrumentation can still fiddle
>>    it out, if desired. Document this behaviour in a half sentence.
>>
>> * Move the hook documentation to where it belongs. Texi file (re-)
>>    generation should work cleanly now.
>>
>> * Documentation clarified as requested.
>>
>>     Torsten
>>
>> [snip]
>>
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 03ba8fc436c..a4c3c98b9f5 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -3105,6 +3105,27 @@ that affect more than one function.
>>   This attribute should be used for debugging purposes only.  It is not
>>   suitable in production code.
>>
>> +@item patchable_function_entry
>> +@cindex @code{patchable_function_entry} function attribute
>> +@cindex extra NOP instructions at the function entry point
>> +In case the target's text segment can be made writable at run time by
>> +any means, padding the function entry with a number of NOPs can be
>> +used to provide a universal tool for instrumentation.
>> +
>> +The @code{patchable_function_entry} function attribute can be used to
>> +change the number of NOPs to any desired value.  The two-value syntax
>> +is the same as for the command-line switch
>> +@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with
>> +the function entry point before the @var{M}th NOP instruction.
>> +@var{M} defaults to 0 if omitted e.g. function entry point is before
>> +the first NOP.
>> +
>> +If patchable function entries are enabled globally using the command
>> +line option @option{-fpatchable-function-entry=N,M}, then all functions
> 
> s/command line option/command-line option
> 
>> +that are part of the instrumentation framework must disable
>> +instrumentation with the attribute @code{patchable_function_entry (0)}
>> +to prevent recursion.
> 
> The functions don't disable instrumentation, programmers disable the
> instrumentation on functions.  Rewrite this clause as
> 
> ...then you must disable instrumentation on all functions that are part
> of the instrumentation framework with the attribute...
> 
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 04cecf94405..1b8a4555b33 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -11520,6 +11520,34 @@ of the function name, it is considered to be
>> a match.  For C99 and C++
>>   extended identifiers, the function name must be given in UTF-8, not
>>   using universal character names.
>>
>> +@item -fpatchable-function-entry=@var{N}[,@var{M}]
>> +@opindex fpatchable-function-entry
>> +Generate @var{N} NOPs right at the beginning
>> +of each function, with the function entry point before the @var{M}th
>> NOP.
>> +If @var{M} is omitted, it defaults to @code{0} so the
>> +function entry points to the address just at the first NOP.
>> +The NOP instructions reserve extra space which can be used to patch in
>> +any desired instrumentation at run time, provided that the code segment
>> +is writable.  The amount of space is controllable indirectly via
>> +the number of NOPs; the NOP instruction used corresponds to the
>> instruction
>> +emitted by the internal GCC back-end interface @code{gen_nop}.  This
>> behavior
>> +is target-specific and may also depend on the architecture variant
>> and/or
>> +other compilation options.
>> +
>> +For run-time identification, the starting addresses of these areas,
>> +which correspond to their respective function entries minus @var{M},
>> +are additionally collected in the @code{__patchable_function_entries}
>> +section of the resulting binary, if the platform supports it.
>> +
>> +Note that the value of @code{__attribute__ ((patchable_function_entry
>> +(N,M)))} takes precedence over command-line option
>> +@option{-fpatchable-function-entry=N,M}.  This can be used to increase
>> +the area size or to remove it completely on a single function.
>> +If @code{N=0}, no pad location is recorded.
>> +
>> +The NOP instructions are inserted at --- and maybe before, depending on
>> +@var{M} --- the function entry address, even before the prologue.
> 
> No spaces around the em-dashes '---'.
> 
>> +DEFHOOK
>> +(print_patchable_function_entry,
>> + "Generate a patchable area at the function start, consisting of\n\
>> +@var{patch_area_size} NOP instructions.  If the target supports named\n\
>> +sections and if @var{record_p} is true, insert a pointer to the
>> current\n\
>> +location in the table of patchable functions.  This table will then
>> be held\n\
>> +in a special section called @code{__patchable_function_entries}.",
> 
> I don't understand when "then" might be.  Can you rewrite this in the
> present tense?  "This table is held..."
> 
> -Sandra
> 


Perhaps you should also mention that this is what the default
implementation of the hook does.  So:

The default implementation of the hook places the table of pointers in
the special section named @code{__patchable_function_entries}.

The point is that another implementation of the hook might put them in a
different section, or even record them entirely differently.

R.