[PATCH v3] add -fprolog-pad=N,M option

[PATCH v3] add -fprolog-pad=N,M option

[Torsten Duwe]

On Tue, Dec 06, 2016 at 05:02:36PM +0300, Maxim Kuvyrkov wrote:
> > On Oct 5, 2016, at 12:45 AM, Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> wrote:
> > 
> > Ideally, I want to improve support for -fprolog-pad=N and __attribute__((prolog_pad(N))) to provide functionality to also output pad before the function label to address use-cases for s390, sparc, etc (what Jose E. Marchesi was referring to).  I.e., -fprolog-pad= option would accept both -fprolog-pad=N and -fprolog-pad=M,N forms -- issue M nops before function label and N nops after function label.  Similarly for __attribute__((prolog_pad(N,M))).  I (or you :-) ) can attempt to implement this functionality before stage1 closes, but it should not block this initial patch.

Implemented :-]

Other changes since v2 are the missing docs added and it is based on the
master branch now. I have deliberately not approached the warning topic
yet; IMHO the generalisation of my use case is quite valid for any
instrumentation: functions part of the instrumentation framework itself
might reside in the same source file as some target functions but need to
be excluded with an __attribute__((prolog_pad(0))). Who knows what else is
legitimate...

Suggested by Richard Biener, this feature is no longer language specific;
only the __attribute__ syntax is of course limited to C.

> >> Changes since the previous version
> >> (which in turn was based on Maxim's suggestion):
> >> 
> >> * Document the feature in *.texi
> >> 
> >> * Automatically disable IPA-RA, like normal profiling does.
> >> You never know in advance what the code patched in at run time will do.
> >> Any optimisation here is potentially wrong.
> >> 
> >> * record a prolog_nop_pad_size value specified on the command line
> >> in each function's attributes, so that it survives an LTO pipe.
> >> 
Signed-off-by: Torsten Duwe <duwe@suse.de>

diff --git a/gcc/attribs.c b/gcc/attribs.c
index e66349a..6ff81a8 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -365,6 +365,28 @@ decl_attributes (tree *node, tree attributes, int flags)
   if (!attributes_initialized)
     init_attributes ();
 
+  /* If we're building NOP pads because of a command line arg, note the size
+     for LTO builds, unless the attribute has already been overridden. */
+  if (TREE_CODE (*node) == FUNCTION_DECL &&
+      prolog_nop_pad_size > 0)
+    {
+      tree pp_attr = lookup_attribute ("prolog_pad", attributes);
+      if (!pp_attr)
+	{
+	  tree pp_size = build_int_cstu (integer_type_node, prolog_nop_pad_size);
+	  tree pp_entry = build_int_cstu (integer_type_node, prolog_nop_pad_entry);
+
+	  attributes = tree_cons (get_identifier ("prolog_pad"),
+				  tree_cons (NULL_TREE,
+					     pp_size,
+					     tree_cons (NULL_TREE,
+							pp_entry,
+							NULL_TREE)
+					     ),
+				  attributes);
+	}
+    }
+
   /* If this is a function and the user used #pragma GCC optimize, add the
      options to the attribute((optimize(...))) list.  */
   if (TREE_CODE (*node) == FUNCTION_DECL && current_optimize_pragma)
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index f5adade..5881d05 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -139,6 +139,7 @@ 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_prolog_pad_attribute (tree *, tree, tree, int, bool *);
 
 /* Table of machine-independent attributes common to all C-like languages.
 
@@ -345,6 +346,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_bnd_instrument, false },
   { "fallthrough",	      0, 0, false, false, false,
 			      handle_fallthrough_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { NULL,                     0, 0, false, false, false, NULL, false }
 };
 
@@ -3173,3 +3176,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
   *no_add_attrs = true;
   return NULL_TREE;
 }
+
+static tree
+handle_prolog_pad_attribute (tree *, tree, tree, int,
+			     bool *)
+{      
+  return NULL_TREE;
+}
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index db293fe..7f3e558 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2322,6 +2322,34 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
       TREE_ASM_WRITTEN (olddecl) = 0;
     }
 
+  /* Prolog pad size may be set wrongly by a forward declaration;
+     fix it up by pulling the final value in front.
+  */
+  if (TREE_CODE (newdecl) == FUNCTION_DECL && new_is_definition &&
+      lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (newdecl)) &&
+      lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (olddecl)))
+    {
+      tree last_pp_attr = NULL_TREE;
+      tree *it;
+
+      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))
+	{
+	  if (IDENTIFIER_LENGTH (get_attribute_name (*it)) !=
+	      strlen("prolog_pad"))
+	    continue;
+	  if (strcmp ("prolog_pad",
+		      IDENTIFIER_POINTER (get_attribute_name (*it))))
+	    continue;
+
+	  last_pp_attr = *it;
+	  *it = TREE_CHAIN(last_pp_attr);
+	  TREE_CHAIN(last_pp_attr) = NULL_TREE;
+	  break;
+	}
+      DECL_ATTRIBUTES (olddecl) = chainon (last_pp_attr,
+					   DECL_ATTRIBUTES (olddecl));
+    }
+
   DECL_ATTRIBUTES (newdecl)
     = targetm.merge_decl_attributes (olddecl, newdecl);
 
diff --git a/gcc/common.opt b/gcc/common.opt
index b350b07..2cb8979 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -163,6 +163,13 @@ bool flag_stack_usage_info = false
 Variable
 int flag_debug_asm
 
+; If we should generate NOP pads before each function prologue
+Variable
+HOST_WIDE_INT prolog_nop_pad_size
+
+; And how far the asm entry point is into this pad
+Variable
+HOST_WIDE_INT prolog_nop_pad_entry
 
 ; Balance between GNAT encodings and standard DWARF to emit.
 Variable
@@ -2019,6 +2026,10 @@ fprofile-reorder-functions
 Common Report Var(flag_profile_reorder_functions)
 Enable function reordering that improves code placement.
 
+fprolog-pad=
+Common Joined
+Pad NOPs before each function prolog
+
 frandom-seed
 Common Var(common_deferred_options) Defer
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index a8402e1..c26ec54 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3072,6 +3072,17 @@ that affect more than one function.
 This attribute should be used for debugging purposes only.  It is not
 suitable in production code.
 
+@item prolog_pad
+@cindex @code{prolog_pad} function attribute
+@cindex function entry padded with NOPs
+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. Usually,
+prolog padding is enabled globally using the -fprolog-pad= command
+line switch, and disabled by the attribute keyword for functions
+that are part of the actual instrumentation framework, to easily avoid
+an endless 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 034ae98..af4f4b0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11207,6 +11207,21 @@ 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 -fprolog-pad=N
+@opindex fprolog-pad
+Generate a pad of N nops right at the beginning
+of each function, which can be used to patch in any desired
+instrumentation at run time, provided that the code segment
+is writeable. For run time identification, the starting addresses
+of these pads, which correspond to their respective functions,
+are additionally collected in the @code{__prolog_pads_loc} section
+of the resulting binary.
+
+Note that value of @code{__attribute__((prolog_pad(N)))} takes
+precedence over command-line option -fprolog_pad=N. This can be used
+to increase the pad size or to remove the pad completely on a single
+function.
+
 @end table
 
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index cdf5f48..65c265c 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4564,6 +4564,10 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
+Generate prologue pad
+@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 bbf53c9..88424ae 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3648,6 +3648,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_PROLOG_PAD
+
 @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 58f6e0c..6723506 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -50,6 +50,7 @@ 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_prolog_pad_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 *);
@@ -78,6 +79,8 @@ const struct attribute_spec lto_attribute_table[] =
 			      handle_nonnull_attribute, false },
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
+  { "prolog_pad",	      1, 2, false, true, true,
+			      handle_prolog_pad_attribute, false },
   { "returns_twice",          0, 0, true,  false, false,
 			      handle_returns_twice_attribute, false },
   { "sentinel",               0, 1, false, true, true,
@@ -475,6 +478,14 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
   return NULL_TREE;
 }
 
+static tree
+handle_prolog_pad_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 c61c367..05cd4bb 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2080,6 +2080,26 @@ common_handle_option (struct gcc_options *opts,
         opts->x_flag_ipa_reference = false;
       break;
 
+    case OPT_fprolog_pad_:
+      {
+	const char *comma = strchr (arg, ',');
+	if (comma)
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = atoi (comma + 1);
+	  }
+	else
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = 0;
+	  }
+	if (prolog_nop_pad_size < 0 ||
+	    prolog_nop_pad_entry < 0 ||
+	    prolog_nop_pad_size < prolog_nop_pad_entry)
+	  error ("invalid arguments for %<-fprolog_pad%>");
+      }
+      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 ac3470e..487b900 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -288,6 +288,12 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
  void, (tree decl, int visibility),
  default_assemble_visibility)
 
+DEFHOOK
+(print_prolog_pad,
+ "Generate prologue pad",
+ void, (FILE *file, unsigned HOST_WIDE_INT pad_size, bool record_p),
+ default_print_prolog_pad)
+
 /* Output the assembler code for entry to a function.  */
 DEFHOOK
 (function_prologue,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 5d3e91e..501a501 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1617,6 +1617,25 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
+void
+default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
+			  bool record_p)
+{
+  if (record_p)
+    fprintf (file, "1:");
+
+  unsigned i;
+  for (i = 0; i < pad_size; ++i)
+    fprintf (file, "\tnop\n");
+
+  if (record_p)
+    {
+      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
+      fprintf (file, "\t.quad 1b\n");
+      fprintf (file, "\t.previous\n");
+    }
+}
+
 bool
 default_profile_before_prologue (void)
 {
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 3a9271f..13c33de 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,7 @@ 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_prolog_pad (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/toplev.c b/gcc/toplev.c
index 5af02ea..d4a46fa 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1595,8 +1595,10 @@ process_options (void)
     }
 
  /* Do not use IPA optimizations for register allocation if profiler is active
+    or prolog-pads 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 || prolog_nop_pad_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 f3cd70a..afa0304 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1830,6 +1830,40 @@ assemble_start_function (tree decl, const char *fnname)
   if (DECL_PRESERVE_P (decl))
     targetm.asm_out.mark_decl_preserved (fnname);
 
+  unsigned HOST_WIDE_INT pad_size = 0;
+  unsigned HOST_WIDE_INT pad_entry = 0;
+
+  tree prolog_pad_attr
+    = lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (decl));
+  if (prolog_pad_attr)
+    {
+      tree pp_val = TREE_VALUE (prolog_pad_attr);
+      tree prolog_pad_value1 = TREE_VALUE (pp_val);
+
+      if (tree_fits_uhwi_p (prolog_pad_value1) )
+	{
+	  pad_size = tree_to_uhwi (prolog_pad_value1);
+	}
+      else
+	gcc_unreachable ();
+
+      if (list_length (pp_val) > 1)
+	{
+	  tree prolog_pad_value2 = TREE_VALUE (TREE_CHAIN (pp_val));
+
+	  if (tree_fits_uhwi_p (prolog_pad_value2))
+	    {
+	      pad_entry = tree_to_uhwi (prolog_pad_value2);
+	    }
+	  else
+	    gcc_unreachable ();
+	}
+    }
+
+  /* Emit the prolog padding before the entry label, if any */
+  if (pad_entry > 0)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_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);
@@ -1838,6 +1872,11 @@ 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 padding after the label. Record it if we haven't done so yet */
+  if (pad_size > pad_entry)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
+				      (pad_entry == 0));
+
   if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
     saw_no_split_stack = true;
 }

Re: [PATCH v3] add -fprolog-pad=N,M option

[Bernd Schmidt]

I'll consider myself agnostic as to whether this is a feature we want or 
need, so I'll just comment on some style questions. There's a fair 
amount of coding style violations, I'll point some of them out but 
please read the documents we have linked on this page:
   https://gcc.gnu.org/contribute.html

On 12/16/2016 03:14 PM, Torsten Duwe wrote:
> Signed-off-by: Torsten Duwe <duwe@suse.de>

This is meaningless for the GCC project. We require a copyright 
assignment; I assume SuSE has a blanket one that covers you. You should 
also write a ChangeLog entry.

> diff --git a/gcc/attribs.c b/gcc/attribs.c
> index e66349a..6ff81a8 100644
> --- a/gcc/attribs.c
> +++ b/gcc/attribs.c
> @@ -365,6 +365,28 @@ decl_attributes (tree *node, tree attributes, int flags)
>    if (!attributes_initialized)
>      init_attributes ();
>
> +  /* If we're building NOP pads because of a command line arg, note the size
> +     for LTO builds, unless the attribute has already been overridden. */

Two spaces at the end of a sentence, including at the end of a comment.

> +  if (TREE_CODE (*node) == FUNCTION_DECL &&
> +      prolog_nop_pad_size > 0)

Operators go on the next line when wrapping.

> +					     ),

Don't put closing parens on their own line.

> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index db293fe..7f3e558 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -2322,6 +2322,34 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
>        TREE_ASM_WRITTEN (olddecl) = 0;
>      }
>
> +  /* Prolog pad size may be set wrongly by a forward declaration;
> +     fix it up by pulling the final value in front.
> +  */

The "*/" should go on the previous line.

> +      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))

Space before opening parentheses (many occurrences).
>
> +void
> +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
> +			  bool record_p)

Every function needs a comment describing its purpose and that of its 
arguments. Look around for examples. There are cases in this patch of 
hook implementations which ignore all their args, there I believe we can 
relax this rule a little (but a comment saying which hook is implemented 
would still be good).

>
> +extern void default_print_prolog_pad (FILE *, unsigned HOST_WIDE_INT , bool);

Careful with stray whitespace.
> +      if (tree_fits_uhwi_p (prolog_pad_value1) )

Here too.
> +	{
> +	  pad_size = tree_to_uhwi (prolog_pad_value1);
> +	}

Lose { } braces around single statements. Several cases.

Am I missing it or is there no actual implementation of the target hook 
anywhere?


Bernd

Re: [PATCH v3] add -fprolog-pad=N,M option

[Maxim Kuvyrkov]

> On Dec 19, 2016, at 6:04 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> 
> I'll consider myself agnostic as to whether this is a feature we want or need,

Hi Bernd, thanks for reviewing this!

Regarding the usefulness of this feature, it has been discussed here (2 years ago): http://gcc.gcc.gnu.narkive.com/JfWUDn8Y/rfc-kernel-livepatching-support-in-gcc .

Kernel live-patching is of interest to several major distros, and it already supported by GCC via architecture-specific implementations (x86, s390, sparc).  The -fprolog-pad= option is an architecture-neutral equivalent that can be used for porting kernel live-patching to new architectures.

Existing support for kernel live-patching in x86, s390 and sparc backends can be redirected to use -fprolog-pad= functionality and, thus, simplified.

--
Maxim Kuvyrkov
www.linaro.org


> so I'll just comment on some style questions. There's a fair amount of coding style violations, I'll point some of them out but please read the documents we have linked on this page:
>  https://gcc.gnu.org/contribute.html
> 
> On 12/16/2016 03:14 PM, Torsten Duwe wrote:
>> Signed-off-by: Torsten Duwe <duwe@suse.de>
> 
> This is meaningless for the GCC project. We require a copyright assignment; I assume SuSE has a blanket one that covers you. You should also write a ChangeLog entry.
> 
>> diff --git a/gcc/attribs.c b/gcc/attribs.c
>> index e66349a..6ff81a8 100644
>> --- a/gcc/attribs.c
>> +++ b/gcc/attribs.c
>> @@ -365,6 +365,28 @@ decl_attributes (tree *node, tree attributes, int flags)
>>   if (!attributes_initialized)
>>     init_attributes ();
>> 
>> +  /* If we're building NOP pads because of a command line arg, note the size
>> +     for LTO builds, unless the attribute has already been overridden. */
> 
> Two spaces at the end of a sentence, including at the end of a comment.
> 
>> +  if (TREE_CODE (*node) == FUNCTION_DECL &&
>> +      prolog_nop_pad_size > 0)
> 
> Operators go on the next line when wrapping.
> 
>> +					     ),
> 
> Don't put closing parens on their own line.
> 
>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
>> index db293fe..7f3e558 100644
>> --- a/gcc/c/c-decl.c
>> +++ b/gcc/c/c-decl.c
>> @@ -2322,6 +2322,34 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
>>       TREE_ASM_WRITTEN (olddecl) = 0;
>>     }
>> 
>> +  /* Prolog pad size may be set wrongly by a forward declaration;
>> +     fix it up by pulling the final value in front.
>> +  */
> 
> The "*/" should go on the previous line.
> 
>> +      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))
> 
> Space before opening parentheses (many occurrences).
>> 
>> +void
>> +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
>> +			  bool record_p)
> 
> Every function needs a comment describing its purpose and that of its arguments. Look around for examples. There are cases in this patch of hook implementations which ignore all their args, there I believe we can relax this rule a little (but a comment saying which hook is implemented would still be good).
> 
>> 
>> +extern void default_print_prolog_pad (FILE *, unsigned HOST_WIDE_INT , bool);
> 
> Careful with stray whitespace.
>> +      if (tree_fits_uhwi_p (prolog_pad_value1) )
> 
> Here too.
>> +	{
>> +	  pad_size = tree_to_uhwi (prolog_pad_value1);
>> +	}
> 
> Lose { } braces around single statements. Several cases.
> 
> Am I missing it or is there no actual implementation of the target hook anywhere?
> 
> 
> Bernd

[PATCH v4] add -fprolog-pad=N,M option

[Torsten Duwe]

On Tue, Dec 20, 2016 at 04:34:02PM +0300, Maxim Kuvyrkov wrote:
> 
> Hi Bernd, thanks for reviewing this!
> 
> Regarding the usefulness of this feature, it has been discussed here (2 years ago): http://gcc.gcc.gnu.narkive.com/JfWUDn8Y/rfc-kernel-livepatching-support-in-gcc .
> 
> Kernel live-patching is of interest to several major distros, and it already supported by GCC via architecture-specific implementations (x86, s390, sparc).  The -fprolog-pad= option is an architecture-neutral equivalent that can be used for porting kernel live-patching to new architectures.
> 
> Existing support for kernel live-patching in x86, s390 and sparc backends can be redirected to use -fprolog-pad= functionality and, thus, simplified.

Yes, indeed.

Hopefully I caught all the style problems, plus the functional change
that I no longer fiddle with the attributes by hand. I was really hoping
for more feedback on the functionality...

To be done: your default_print_prolog_pad really nicely handles >90% of
all cases, but what if a) the target platform does not allow arbitrary
section names or b) it has very distinct ideas about code sizes and offsets
around the prologue? (All target CPUs have an insn called "nop" which
does what we expect, AFAICS ;-) gcc should error out somehow meaningfully,
IMHO.

	Torsten

2016-12-21	Torsten Duwe	<duwe@suse.de> : 

	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
	   a handler for it.

	 * lto/lto-lang.c : Likewise.

	 * common.opt : introduce -fprolog_pad command line option
	   and its variables prolog_nop_pad_size and prolog_nop_pad_entry.

	 * doc/extend.texi : document prolog_pad attribute.

	 * doc/invoke.texi : document -fprolog_pad command line option.

	 * opts.c (OPT_fprolog_pad_): add parser.

	 * doc/tm.texi.in (TARGET_ASM_PRINT_PROLOG_PAD): new target hook

	 * doc/tm.texi : Likewise.

	 * target.def (print_prolog_pad): Likewise.

	 * targhooks.h (default_print_prolog_pad): new function.

	 * targhooks.c (default_print_prolog_pad): Likewise.

	 * testsuite/c-c++-common/attribute-prolog_pad-1.c : New test.

	 * toplev.c (process_options): Switch off IPA-RA if
	   prolog pads are being generated.

	 * varasm.c (assemble_start_function): look at prolog-pad command
	   line switch and function attributes and maybe generate nop
	   pads by calling print_prolog_pad.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index f5adade..21d0386 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -139,6 +139,7 @@ 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_prolog_pad_attribute (tree *, tree, tree, int, bool *);
 
 /* Table of machine-independent attributes common to all C-like languages.
 
@@ -345,6 +346,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_bnd_instrument, false },
   { "fallthrough",	      0, 0, false, false, false,
 			      handle_fallthrough_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { NULL,                     0, 0, false, false, false, NULL, false }
 };
 
@@ -3173,3 +3176,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
   *no_add_attrs = true;
   return NULL_TREE;
 }
+
+static tree
+handle_prolog_pad_attribute (tree *, tree, tree, int,
+			     bool *)
+{
+  return NULL_TREE;
+}
diff --git a/gcc/common.opt b/gcc/common.opt
index de06844..0317a86 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -163,6 +163,13 @@ bool flag_stack_usage_info = false
 Variable
 int flag_debug_asm
 
+; If we should generate NOP pads before each function prologue
+Variable
+HOST_WIDE_INT prolog_nop_pad_size
+
+; And how far the asm entry point is into this pad
+Variable
+HOST_WIDE_INT prolog_nop_pad_entry
 
 ; Balance between GNAT encodings and standard DWARF to emit.
 Variable
@@ -2019,6 +2026,10 @@ fprofile-reorder-functions
 Common Report Var(flag_profile_reorder_functions)
 Enable function reordering that improves code placement.
 
+fprolog-pad=
+Common Joined Optimization
+Pad NOPs before each function prolog
+
 frandom-seed
 Common Var(common_deferred_options) Defer
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 1f303bc..a09851a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3076,6 +3076,17 @@ that affect more than one function.
 This attribute should be used for debugging purposes only.  It is not
 suitable in production code.
 
+@item prolog_pad
+@cindex @code{prolog_pad} function attribute
+@cindex function entry padded with NOPs
+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.  Usually,
+prolog padding is enabled globally using the -fprolog-pad= command
+line switch, and disabled by the attribute keyword for functions
+that are part of the actual instrumentation framework, to easily avoid
+an endless 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 b729964..21e5067 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11309,6 +11309,21 @@ 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 -fprolog-pad=N
+@opindex fprolog-pad
+Generate a pad of N nops right at the beginning
+of each function, which can be used to patch in any desired
+instrumentation at run time, provided that the code segment
+is writeable.  For run time identification, the starting addresses
+of these pads, which correspond to their respective functions,
+are additionally collected in the @code{__prolog_pads_loc} section
+of the resulting binary.
+
+Note that value of @code{__attribute__ ((prolog_pad (N)))} takes
+precedence over command-line option -fprolog_pad=N.  This can be used
+to increase the pad size or to remove the pad completely on a single
+function.
+
 @end table
 
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index cdf5f48..65c265c 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4564,6 +4564,10 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
+Generate prologue pad
+@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 bbf53c9..88424ae 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3648,6 +3648,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_PROLOG_PAD
+
 @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 58f6e0c..94599ea 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -50,6 +50,7 @@ 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_prolog_pad_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 *);
@@ -78,6 +79,8 @@ const struct attribute_spec lto_attribute_table[] =
 			      handle_nonnull_attribute, false },
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { "returns_twice",          0, 0, true,  false, false,
 			      handle_returns_twice_attribute, false },
   { "sentinel",               0, 1, false, true, true,
@@ -475,6 +478,14 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
   return NULL_TREE;
 }
 
+static tree
+handle_prolog_pad_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 890da03..af3a5b5 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2080,6 +2080,26 @@ common_handle_option (struct gcc_options *opts,
         opts->x_flag_ipa_reference = false;
       break;
 
+    case OPT_fprolog_pad_:
+      {
+	const char *comma = strchr (arg, ',');
+	if (comma)
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = atoi (comma + 1);
+	  }
+	else
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = 0;
+	  }
+	if (prolog_nop_pad_size < 0
+	    || prolog_nop_pad_entry < 0
+	    || prolog_nop_pad_size < prolog_nop_pad_entry)
+	  error ("invalid arguments for %<-fprolog_pad%>");
+      }
+      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 ac3470e..487b900 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -288,6 +288,12 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
  void, (tree decl, int visibility),
  default_assemble_visibility)
 
+DEFHOOK
+(print_prolog_pad,
+ "Generate prologue pad",
+ void, (FILE *file, unsigned HOST_WIDE_INT pad_size, bool record_p),
+ default_print_prolog_pad)
+
 /* Output the assembler code for entry to a function.  */
 DEFHOOK
 (function_prologue,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 5d3e91e..b6de992 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1617,6 +1617,31 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
+/* Write pad_size NOPs into the asm outfile before a function
+   prologue.  If record_p is true, the location of the pad will be
+   recorded in a special object section called "__prolog_pads_loc".
+   This routine may be called twice per function to put NOPs before
+   and after the function entry.  */
+
+void
+default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
+			  bool record_p)
+{
+  if (record_p)
+    fprintf (file, "1:");
+
+  unsigned i;
+  for (i = 0; i < pad_size; ++i)
+    fprintf (file, "\tnop\n");
+
+  if (record_p)
+    {
+      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
+      fprintf (file, "\t.quad 1b\n");
+      fprintf (file, "\t.previous\n");
+    }
+}
+
 bool
 default_profile_before_prologue (void)
 {
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 3a9271f..13c33de 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,7 @@ 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_prolog_pad (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/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
new file mode 100644
index 0000000..179b492
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fprolog-pad=3,1" } */
+
+void f1 (void) __attribute__((prolog_pad(2,1)));
+void f2 (void) __attribute__((prolog_pad(3)));
+int f3 (void);
+
+void
+f1 (void)
+{
+  f2 ();
+}
+
+void
+f2 (void)
+{
+  f1 ();
+}
+
+/* F3 should never have a nop pad.  */
+int
+__attribute__((prolog_pad(0)))
+__attribute__((noinline))
+f3 (void)
+{
+  return 5;
+}
+
+/* F4 should receive the command line default setting.  */
+int
+f4 (void)
+{
+  return 3*f3 ()+1;
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 79d7a6f..ab4578e 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1595,8 +1595,10 @@ process_options (void)
     }
 
  /* Do not use IPA optimizations for register allocation if profiler is active
+    or prolog pads 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 || prolog_nop_pad_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 5b15847..c1308f2 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1830,6 +1830,44 @@ assemble_start_function (tree decl, const char *fnname)
   if (DECL_PRESERVE_P (decl))
     targetm.asm_out.mark_decl_preserved (fnname);
 
+  unsigned HOST_WIDE_INT pad_size = prolog_nop_pad_size;
+  unsigned HOST_WIDE_INT pad_entry = prolog_nop_pad_entry;
+
+  tree prolog_pad_attr
+    = lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (decl));
+  if (prolog_pad_attr)
+    {
+      tree pp_val = TREE_VALUE (prolog_pad_attr);
+      tree prolog_pad_value1 = TREE_VALUE (pp_val);
+
+      if (tree_fits_uhwi_p (prolog_pad_value1))
+	pad_size = tree_to_uhwi (prolog_pad_value1);
+      else
+	gcc_unreachable ();
+
+      pad_entry = 0;
+      if (list_length (pp_val) > 1)
+	{
+	  tree prolog_pad_value2 = TREE_VALUE (TREE_CHAIN (pp_val));
+
+	  if (tree_fits_uhwi_p (prolog_pad_value2))
+	    pad_entry = tree_to_uhwi (prolog_pad_value2);
+	  else
+	    gcc_unreachable ();
+	}
+    }
+
+  if (pad_entry > pad_size)
+    {
+      if (pad_size > 0)
+	warning (OPT_Wattributes, "Prolog nop pad entry > size");
+      pad_entry = 0;
+    }
+
+  /* Emit the prolog padding before the entry label, if any.  */
+  if (pad_entry > 0)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_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);
@@ -1838,6 +1876,11 @@ 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 padding after the label.  Record it if we haven't done so yet.  */
+  if (pad_size > pad_entry)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
+				      (pad_entry == 0));
+
   if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
     saw_no_split_stack = true;
 }

Re: [PATCH v4] add -fprolog-pad=N,M option

[Sandra Loosemore]

On 12/21/2016 10:23 AM, Torsten Duwe wrote:

> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 1f303bc..a09851a 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3076,6 +3076,17 @@ that affect more than one function.
>   This attribute should be used for debugging purposes only.  It is not
>   suitable in production code.
>
> +@item prolog_pad
> +@cindex @code{prolog_pad} function attribute
> +@cindex function entry padded with NOPs
> +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.  Usually,
> +prolog padding is enabled globally using the -fprolog-pad= command
> +line switch, and disabled by the attribute keyword for functions

@option{-fprolog-pad=} command-line switch

> +that are part of the actual instrumentation framework, to easily avoid
> +an endless recursion.

I'm confused.  Does the prolog_pad attribute *disable* the padding, 
then?  You should say that explicitly.

>   @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 b729964..21e5067 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11309,6 +11309,21 @@ 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 -fprolog-pad=N

@var{N}

> +@opindex fprolog-pad
> +Generate a pad of N nops right at the beginning

@var{N} again.

Can we be consistent about whether it's "NOP" or "nop"?  You used "NOP" 
in the doc snippet above, and that seems to be the preferred usage.

> +of each function, which can be used to patch in any desired

What if the target supports NOPs of different sizes?  (E.g., nios2 with 
-march=r2 -mcdx).

Where, exactly, is the padding inserted?  "At the beginning of each 
function" might correspond to the address of the first instruction of 
the function, or it might correspond to the address of some instruction 
after the prolog where GDB would set a breakpoint on function entry. 
This feature isn't usable without more explicit documentation about what 
it does.

> +instrumentation at run time, provided that the code segment
> +is writeable.  For run time identification, the starting addresses

s/writeable/writable/
s/run time identification/run-time identification/

> +of these pads, which correspond to their respective functions,
> +are additionally collected in the @code{__prolog_pads_loc} section
> +of the resulting binary.
> +
> +Note that value of @code{__attribute__ ((prolog_pad (N)))} takes

You didn't explain this syntax in the documentation of the attribute 
above...

> +precedence over command-line option -fprolog_pad=N.  This can be used

@option and @var markup on the option, please.  And if N here is 
different than N above, don't re-use the same metasyntactic variable for it.

> +to increase the pad size or to remove the pad completely on a single
> +function.
> +
>   @end table
>
>
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index cdf5f48..65c265c 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -4564,6 +4564,10 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
> +Generate prologue pad

Needs more extensive documentation, and sentences should end in a period.

-Sandra

[PATCH v5] add -fprolog-pad=N,M option

[Torsten Duwe]

Changes since v4: hopefully addressed all of Sandra's requests
and suggestions concerning the documentation snippets, thanks
for the feedback.  If it still isn't clear, feel free to rephrase
-- I'm a programmer, not a technical writer.

2017-01-13	Torsten Duwe	<duwe@suse.de> : 

	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
	   a handler for it.

	 * lto/lto-lang.c : Likewise.

	 * common.opt : introduce -fprolog_pad command line option
	   and its variables prolog_nop_pad_size and prolog_nop_pad_entry.

	 * doc/extend.texi : document prolog_pad attribute.

	 * doc/invoke.texi : document -fprolog_pad command line option.

	 * opts.c (OPT_fprolog_pad_): add parser.

	 * doc/tm.texi.in (TARGET_ASM_PRINT_PROLOG_PAD): new target hook

	 * doc/tm.texi : Likewise.

	 * target.def (print_prolog_pad): Likewise.

	 * targhooks.h (default_print_prolog_pad): new function.

	 * targhooks.c (default_print_prolog_pad): Likewise.

	 * testsuite/c-c++-common/attribute-prolog_pad-1.c : New test.

	 * toplev.c (process_options): Switch off IPA-RA if
	   prolog pads are being generated.

	 * varasm.c (assemble_start_function): look at prolog-pad command
	   line switch and function attributes and maybe generate NOP
	   pads by calling print_prolog_pad.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index ce7fcaa..5c6cf1c 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -139,6 +139,7 @@ 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_prolog_pad_attribute (tree *, tree, tree, int, bool *);
 
 /* Table of machine-independent attributes common to all C-like languages.
 
@@ -345,6 +346,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_bnd_instrument, false },
   { "fallthrough",	      0, 0, false, false, false,
 			      handle_fallthrough_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { NULL,                     0, 0, false, false, false, NULL, false }
 };
 
@@ -3173,3 +3176,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
   *no_add_attrs = true;
   return NULL_TREE;
 }
+
+static tree
+handle_prolog_pad_attribute (tree *, tree, tree, int,
+			     bool *)
+{
+  return NULL_TREE;
+}
diff --git a/gcc/common.opt b/gcc/common.opt
index 8ad5b77..37d4009 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -163,6 +163,13 @@ bool flag_stack_usage_info = false
 Variable
 int flag_debug_asm
 
+; If we should generate NOP pads before each function prologue
+Variable
+HOST_WIDE_INT prolog_nop_pad_size
+
+; And how far the asm entry point is into this pad
+Variable
+HOST_WIDE_INT prolog_nop_pad_entry
 
 ; Balance between GNAT encodings and standard DWARF to emit.
 Variable
@@ -2019,6 +2026,10 @@ fprofile-reorder-functions
 Common Report Var(flag_profile_reorder_functions)
 Enable function reordering that improves code placement.
 
+fprolog-pad=
+Common Joined Optimization
+Pad NOPs before each function prologue.
+
 frandom-seed
 Common Var(common_deferred_options) Defer
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 6be113c..fb7d62d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3076,6 +3076,23 @@ that affect more than one function.
 This attribute should be used for debugging purposes only.  It is not
 suitable in production code.
 
+@item prolog_pad
+@cindex @code{prolog_pad} function attribute
+@cindex function entry padded with NOPs
+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.  Usually,
+prolog padding is enabled globally using the @option{-fprolog-pad=N,M}
+command-line switch, and disabled with attribute @code{prolog_pad (0)}
+for functions that are part of the actual instrumentation framework.
+This conveniently avoids an endless recursion.
+Of course the @code{prolog_pad} function attribute can be used to
+change the pad size to any desired value.  The two-value syntax is
+the same as for the command-line switch @option{-fprolog-pad=N,M},
+generating a NOP pad of size @var{N}, with the function entry point
+@var{M} NOP instructions into the pad.  @var{M} defaults to 0
+if omitted e.g. function entry point is the beginning of the pad.
+
 @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 69f0adb..d2e11bc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11341,6 +11341,27 @@ 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 -fprolog-pad=@var{N},@var{M}
+@opindex fprolog-pad
+Generate a pad of @var{N} NOPs right at the beginning
+of each function, with the function entry point @var{M} NOPs into
+the pad, which can be used to patch in any desired
+instrumentation at run time, provided that the code segment
+is writable.
+For run-time identification, the starting addresses
+of these pads, which correspond to their respective function entries
+minus @var{M}, are additionally collected in the @code{__prolog_pads_loc}
+section of the resulting binary.
+
+Note that value of @code{__attribute__ ((prolog_pad (N,M)))} takes
+precedence over command-line option @option{-fprolog-pad=N,M}.
+This can be used to increase the pad size or to remove the pad completely
+on a single function.  If @code{N=0}, no pad location is recorded.
+
+The NOP pad is inserted at (and maybe before) the function entry address,
+even before the prologue.  If @var{M} is omitted, it defaults to 0 e.g. the
+pad starts exactly at the function entry point and not before.
+
 @end table
 
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 4b62e05..eb29f71 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4564,6 +4564,16 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
+Generate an array of @var{pad_size} NOP instructions in the
+output @var{file}.  If the target supports NOPs of different sizes,
+the instruction with the smallest code size should be used, in order
+to achieve the finest granularity for @var{pad_size}.  If @var{record_p}
+is true, the current location should be recorded in the
+@code{__prolog_pads_loc} section or a target-specific equivalent.
+
+@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 ea74d37..7b01026 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3648,6 +3648,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_PROLOG_PAD
+
 @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 fccb8c6..d4955c0 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -50,6 +50,7 @@ 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_prolog_pad_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 *);
@@ -78,6 +79,8 @@ const struct attribute_spec lto_attribute_table[] =
 			      handle_nonnull_attribute, false },
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { "returns_twice",          0, 0, true,  false, false,
 			      handle_returns_twice_attribute, false },
   { "sentinel",               0, 1, false, true, true,
@@ -475,6 +478,14 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
   return NULL_TREE;
 }
 
+static tree
+handle_prolog_pad_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 5f573a1..60d80aa 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2157,6 +2157,26 @@ common_handle_option (struct gcc_options *opts,
         opts->x_flag_ipa_reference = false;
       break;
 
+    case OPT_fprolog_pad_:
+      {
+	const char *comma = strchr (arg, ',');
+	if (comma)
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = atoi (comma + 1);
+	  }
+	else
+	  {
+	    prolog_nop_pad_size = atoi (arg);
+	    prolog_nop_pad_entry = 0;
+	  }
+	if (prolog_nop_pad_size < 0
+	    || prolog_nop_pad_entry < 0
+	    || prolog_nop_pad_size < prolog_nop_pad_entry)
+	  error ("invalid arguments for %<-fprolog_pad%>");
+      }
+      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 0443390..60e56cd 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -288,6 +288,12 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
  void, (tree decl, int visibility),
  default_assemble_visibility)
 
+DEFHOOK
+(print_prolog_pad,
+ "Generate prologue pad",
+ void, (FILE *file, unsigned HOST_WIDE_INT pad_size, bool record_p),
+ default_print_prolog_pad)
+
 /* Output the assembler code for entry to a function.  */
 DEFHOOK
 (function_prologue,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 2f2abd3..2ee5e38 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1617,6 +1617,31 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
+/* Write pad_size NOPs into the asm outfile before a function
+   prologue.  If record_p is true, the location of the pad will be
+   recorded in a special object section called "__prolog_pads_loc".
+   This routine may be called twice per function to put NOPs before
+   and after the function entry.  */
+
+void
+default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
+			  bool record_p)
+{
+  if (record_p)
+    fprintf (file, "1:");
+
+  unsigned i;
+  for (i = 0; i < pad_size; ++i)
+    fprintf (file, "\tnop\n");
+
+  if (record_p)
+    {
+      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
+      fprintf (file, "\t.quad 1b\n");
+      fprintf (file, "\t.previous\n");
+    }
+}
+
 bool
 default_profile_before_prologue (void)
 {
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index a5565f5..e302e8d 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,7 @@ 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_prolog_pad (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/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
new file mode 100644
index 0000000..2236aa8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fprolog-pad=3,1" } */
+
+void f1 (void) __attribute__((prolog_pad(2,1)));
+void f2 (void) __attribute__((prolog_pad(3)));
+int f3 (void);
+
+void
+f1 (void)
+{
+  f2 ();
+}
+
+void
+f2 (void)
+{
+  f1 ();
+}
+
+/* F3 should never have a NOP pad.  */
+int
+__attribute__((prolog_pad(0)))
+__attribute__((noinline))
+f3 (void)
+{
+  return 5;
+}
+
+/* F4 should receive the command line default setting.  */
+int
+f4 (void)
+{
+  return 3*f3 ()+1;
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index c0f1a2d..649c4bd 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1595,8 +1595,10 @@ process_options (void)
     }
 
  /* Do not use IPA optimizations for register allocation if profiler is active
+    or prolog pads 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 || prolog_nop_pad_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 11a8ac4..6c40bce 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1830,6 +1830,44 @@ assemble_start_function (tree decl, const char *fnname)
   if (DECL_PRESERVE_P (decl))
     targetm.asm_out.mark_decl_preserved (fnname);
 
+  unsigned HOST_WIDE_INT pad_size = prolog_nop_pad_size;
+  unsigned HOST_WIDE_INT pad_entry = prolog_nop_pad_entry;
+
+  tree prolog_pad_attr
+    = lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (decl));
+  if (prolog_pad_attr)
+    {
+      tree pp_val = TREE_VALUE (prolog_pad_attr);
+      tree prolog_pad_value1 = TREE_VALUE (pp_val);
+
+      if (tree_fits_uhwi_p (prolog_pad_value1))
+	pad_size = tree_to_uhwi (prolog_pad_value1);
+      else
+	gcc_unreachable ();
+
+      pad_entry = 0;
+      if (list_length (pp_val) > 1)
+	{
+	  tree prolog_pad_value2 = TREE_VALUE (TREE_CHAIN (pp_val));
+
+	  if (tree_fits_uhwi_p (prolog_pad_value2))
+	    pad_entry = tree_to_uhwi (prolog_pad_value2);
+	  else
+	    gcc_unreachable ();
+	}
+    }
+
+  if (pad_entry > pad_size)
+    {
+      if (pad_size > 0)
+	warning (OPT_Wattributes, "Prolog nop pad entry > size");
+      pad_entry = 0;
+    }
+
+  /* Emit the prolog padding before the entry label, if any.  */
+  if (pad_entry > 0)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_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);
@@ -1838,6 +1876,11 @@ 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 padding after the label.  Record it if we haven't done so yet.  */
+  if (pad_size > pad_entry)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
+				      (pad_entry == 0));
+
   if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
     saw_no_split_stack = true;
 }

Re: [PATCH v5] add -fprolog-pad=N,M option

[Sandra Loosemore]

On 01/13/2017 05:19 AM, Torsten Duwe wrote:
> Changes since v4: hopefully addressed all of Sandra's requests
> and suggestions concerning the documentation snippets, thanks
> for the feedback.  If it still isn't clear, feel free to rephrase
> -- I'm a programmer, not a technical writer.

Thanks, this makes more sense to me now.  :-)

-Sandra

Re: [PATCH v5] add -fprolog-pad=N,M option

[Torsten Duwe]

And what's next now?

Maxim mentioned in this thread you should sort of formally approve(?)
a global feature, have you had a look at it yet?

I should emphasize that this code can substitute the -fentry for arm64 for
the purpose of kernel live patching that was discussed in the beginning,
as well as for some other architectures, plus the yet-to-be-discovered use
cases as a general instrumentation. In other words, it could save some work
in the future :-)

	Torsten

Re: [PATCH v5] add -fprolog-pad=N,M option

[Bernd Schmidt]

There's still a a few details that need addressing, and some questions I 
have. Also Ccing Jakub to have another pair of eyes on the name of the 
section - I don't know if we want some sort of .gnu.something name.

On 01/13/2017 01:19 PM, Torsten Duwe wrote:
>
> 2017-01-13	Torsten Duwe	<duwe@suse.de> :

First, some people seem to care, so I'll have to ask to please check the 
correct formatting of this line (spacing and all) by looking at existing 
entries. Also remove some of the blank lines in the ChangeLog, you could 
still group the doc entries separately from the code ones.

> 	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
> 	   a handler for it.
>
> 	 * lto/lto-lang.c : Likewise.

 > 	 * testsuite/c-c++-common/attribute-prolog_pad-1.c : New test.

These three directories have separate ChangeLogs. The top-level 
directory name should be stripped and the entries added to the 
appropriate file.

> +      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))
> +	{
> +	  if (IDENTIFIER_LENGTH (get_attribute_name (*it)) !=
> +	      strlen("prolog_pad"))

Still several instances of bad formatting, in the entire block of code. 
Please refer to my previous email.

> +static tree
> +handle_prolog_pad_attribute (tree *, tree, tree, int,
> +			     bool *)
> +{
> +  /* Nothing to be done here.  */
> +  return NULL_TREE;
> +}

Should still add at least one-liner comments before these empty 
functions to say what interface they're implementing. Also, probably 
don't need to wrap the line containing the arguments.

> diff --git a/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> new file mode 100644
> index 0000000..2236aa8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fprolog-pad=3,1" } */

This test does not actually seem to verify that the padding has the 
expected size.

>   /* Do not use IPA optimizations for register allocation if profiler is active
> +    or prolog pads 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 || prolog_nop_pad_size
> +      || !targetm.have_prologue () || !targetm.have_epilogue ())
>      flag_ipa_ra = 0;

Was this explained? Why would ipa-ra be a problem?

> +  if (pad_size > pad_entry)
> +    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
> +				      (pad_entry == 0));

Unnecessary parens around the last argument, and missing spaces around 
operators.


Bernd

Re: [PATCH v5] add -fprolog-pad=N,M option

[Torsten Duwe]

On Mon, Jan 23, 2017 at 05:25:45PM +0100, Bernd Schmidt wrote:
> There's still a a few details that need addressing, and some questions I
> have. Also Ccing Jakub to have another pair of eyes on the name of the
> section - I don't know if we want some sort of .gnu.something name.

So Jakub, Richard E., what's the outcome?

Is this all the feedback the "gcc developers" will provide? Bernd, don't
get me wrong, I also consider it important to watch the coding style in a 
project, I've seen cases were nobody cared ... not good.

But OTOH some of the code you rightfully criticised in the previous run
just went away after some smart feedback from Richard Biener (thanks!);
I'd rather sort out the design issues first before I polish the code.

So, can I please have a statement from a responsible maintainer (if such
a person really exists for this area) about whether this feature would be
acceptable at all?

Thank you very much in advance.

> >diff --git a/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> >new file mode 100644
> >index 0000000..2236aa8
> >--- /dev/null
> >+++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> >@@ -0,0 +1,34 @@
> >+/* { dg-do compile } */
> >+/* { dg-options "-fprolog-pad=3,1" } */
> 
> This test does not actually seem to verify that the padding has the expected
> size.

Honestly, I do not know how to complete that test case, this is a mere sketch.
Shame on me I'm really not familiar with that framework; suggestions welcome.

> >  /* Do not use IPA optimizations for register allocation if profiler is active
> >+    or prolog pads 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 || prolog_nop_pad_size
> >+      || !targetm.have_prologue () || !targetm.have_epilogue ())
> >     flag_ipa_ra = 0;
> 
> Was this explained? Why would ipa-ra be a problem?

I thought that was obvious, sorry. Instrumentation / profiling will need
some registers, and live patching (the current use case) may allocate
different registers for the replacement functions. You will probably want
to replace the NOPs with a standardised code snippet etc. Conclusion: do not
make any assumptions about callee's register utilisation and strictly stick
to the ABI! I remember to have discussed this, maybe not here.

	Torsten

Re: [PATCH v5] add -fprolog-pad=N,M option

[Jakub Jelinek]

On Wed, Feb 08, 2017 at 12:18:05PM +0100, Torsten Duwe wrote:
> On Mon, Jan 23, 2017 at 05:25:45PM +0100, Bernd Schmidt wrote:
> > There's still a a few details that need addressing, and some questions I
> > have. Also Ccing Jakub to have another pair of eyes on the name of the
> > section - I don't know if we want some sort of .gnu.something name.
> 
> So Jakub, Richard E., what's the outcome?
> 
> Is this all the feedback the "gcc developers" will provide? Bernd, don't
> get me wrong, I also consider it important to watch the coding style in a 
> project, I've seen cases were nobody cared ... not good.
> 
> But OTOH some of the code you rightfully criticised in the previous run
> just went away after some smart feedback from Richard Biener (thanks!);
> I'd rather sort out the design issues first before I polish the code.
> 
> So, can I please have a statement from a responsible maintainer (if such
> a person really exists for this area) about whether this feature would be
> acceptable at all?

First of all, GCC is in stage4 and this isn't a fix for a regression, so it
is not acceptable at this point.  Next stage1 will start in April or so.
Second thing that is questionable is what units are the arguments of the
option or attribute.
+  if (record_p)                                                                                                                                   
+    fprintf (file, "1:");                                                                                                                         
+                                                                                                                                                  
+  unsigned i;                                                                                                                                     
+  for (i = 0; i < pad_size; ++i)                                                                                                                  
+    fprintf (file, "\tnop\n");                                                                                                                    
+                                                                                                                                                  
+  if (record_p)                                                                                                                                   
+    {                                                                                                                                             
+      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");                                                                          
+      fprintf (file, "\t.quad 1b\n");                                                                                                             
+      fprintf (file, "\t.previous\n");                                                                                                            
+    }                                                                                                                                             
suggests that it is at least by default the number of nops, whatever that
means.  The length of nop varies greatly across different architectures,
some architectures have different spelling for it (e.g.
ia64, s390{,x} use nop 0 and mmix uses swym 0), some architectures have
tons of different nops with different sizes (see e.g. x86_64/i686), for some
longer sizes it is often beneficial to emit a jump around the rest of nops
instead of just tons of nops.

Even if it is counted always in nops and only nops are emitted (not really
efficient, and I guess kernel cares about efficiency), the above is
definitely not acceptable for a generic hook implementation, it contains too
many ELF specific details as well as 64-bit target specific details etc.
For the label, you should use something like:
  static int ppad_no;
  char tmp_label[...];
  ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LPPAD", ppad_no);
  ppad_no++;
  ASM_OUTPUT_LABEL (file, tmp_label);
the "a",@progbits is not generic enough, you want
  switch_to_section (get_section ("__prolog_pads_loc", 0, NULL));
or so.  .previous doesn't work on many targets, you need to actuall switch
to the previously current section.  .quad is not supported by many
assemblers, you want assemble_integer, with the appropriate size (say
derived from size of pointer), decide whether in that section everything is
aligned or unaligned, emit aligning directive if the former.

> I thought that was obvious, sorry. Instrumentation / profiling will need
> some registers, and live patching (the current use case) may allocate
> different registers for the replacement functions. You will probably want
> to replace the NOPs with a standardised code snippet etc. Conclusion: do not
> make any assumptions about callee's register utilisation and strictly stick
> to the ABI! I remember to have discussed this, maybe not here.

There are many other ways in which the ABI can check, e.g.
see how i386.c (ix86_function_regparm) can change ABI if
          cgraph_local_info *i = &target->local;
          if (i && i->local && i->can_change_signature)
So you'd e.g. need to make sure that functions you emit the padding for
can't change signature.  Bet you won't be able to handle e.g. IPA cp or many
other IPA optimizations that change signature too, while the function from
the source may still be around, perhaps nothing will call it because the
callers have been changed to call its clone with different arguments.
Or say IPA-ICF, if you want to live-patch only some function and not another
function which happens to have identical bogy in the end.

	Jakub

Re: [PATCH v5] add -fprolog-pad=N,M option

[Richard Earnshaw (lists)]

On 13/01/17 12:19, Torsten Duwe wrote:
> Changes since v4: hopefully addressed all of Sandra's requests
> and suggestions concerning the documentation snippets, thanks
> for the feedback.  If it still isn't clear, feel free to rephrase
> -- I'm a programmer, not a technical writer.
> 

Generally, I find 'pad' somewhat confusing.  It's OK for the option name
itself, but more generally, we should be talking about either space or
number of instructions according to context.

> 2017-01-13	Torsten Duwe	<duwe@suse.de> : 

Two spaces between date and name, two more between name and email, no
colon at the end.

> 
> 	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
> 	   a handler for it.
> 


Don't leave blank lines between files mentioned here.  All lines should
be indented by /exactly/ one tab; don't add extra indentation for
continuation lines.  No space before colon.  Capital letter at start of
sentences, full stop at the end of each one.  Which function, or data
structure did you change (put the name in brackets before the colon)?
Document each function or variable changed.

The above entry should read:

	* c-family/c-attribs.c (c_common_attribute_table): Add entry
	for "prolog_pad".
	(handle_prolog_pad_attribute): New function.

> 	 * lto/lto-lang.c : Likewise.
You still need to name the objects affected, even if you can then use
likewise to refer to the immediately preceding entry.

> 
> 	 * common.opt : introduce -fprolog_pad command line option
> 	   and its variables prolog_nop_pad_size and prolog_nop_pad_entry.
> 
> 	 * doc/extend.texi : document prolog_pad attribute.
> 
> 	 * doc/invoke.texi : document -fprolog_pad command line option.
> 
> 	 * opts.c (OPT_fprolog_pad_): add parser.
> 
> 	 * doc/tm.texi.in (TARGET_ASM_PRINT_PROLOG_PAD): new target hook
> 
> 	 * doc/tm.texi : Likewise.
> 
> 	 * target.def (print_prolog_pad): Likewise.
> 
> 	 * targhooks.h (default_print_prolog_pad): new function.
> 
> 	 * targhooks.c (default_print_prolog_pad): Likewise.
> 
> 	 * testsuite/c-c++-common/attribute-prolog_pad-1.c : New test.
> 
> 	 * toplev.c (process_options): Switch off IPA-RA if
> 	   prolog pads are being generated.
> 
> 	 * varasm.c (assemble_start_function): look at prolog-pad command
> 	   line switch and function attributes and maybe generate NOP
> 	   pads by calling print_prolog_pad.
> 
> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
> index ce7fcaa..5c6cf1c 100644
> --- a/gcc/c-family/c-attribs.c
> +++ b/gcc/c-family/c-attribs.c
> @@ -139,6 +139,7 @@ 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_prolog_pad_attribute (tree *, tree, tree, int, bool *);
>  
>  /* Table of machine-independent attributes common to all C-like languages.
>  
> @@ -345,6 +346,8 @@ const struct attribute_spec c_common_attribute_table[] =
>  			      handle_bnd_instrument, false },
>    { "fallthrough",	      0, 0, false, false, false,
>  			      handle_fallthrough_attribute, false },
> +  { "prolog_pad",	      1, 2, true, false, false,
> +			      handle_prolog_pad_attribute, false },
>    { NULL,                     0, 0, false, false, false, NULL, false }
>  };
>  
> @@ -3173,3 +3176,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
>    *no_add_attrs = true;
>    return NULL_TREE;
>  }
> +
> +static tree
> +handle_prolog_pad_attribute (tree *, tree, tree, int,
> +			     bool *)
> +{
> +  return NULL_TREE;
> +}
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 8ad5b77..37d4009 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -163,6 +163,13 @@ bool flag_stack_usage_info = false
>  Variable
>  int flag_debug_asm
>  
> +; If we should generate NOP pads before each function prologue

'If' suggests this is a boolean, but the declaration is for a count, so
this should be "Number of NOP instructions to insert before each
function prologue."

> +Variable
> +HOST_WIDE_INT prolog_nop_pad_size
> +
> +; And how far the asm entry point is into this pad
> +Variable
> +HOST_WIDE_INT prolog_nop_pad_entry
>  
>  ; Balance between GNAT encodings and standard DWARF to emit.
>  Variable
> @@ -2019,6 +2026,10 @@ fprofile-reorder-functions
>  Common Report Var(flag_profile_reorder_functions)
>  Enable function reordering that improves code placement.
>  
> +fprolog-pad=
> +Common Joined Optimization
> +Pad NOPs before each function prologue.
> +

Insert NOP instructions before ...

>  frandom-seed
>  Common Var(common_deferred_options) Defer
>  
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 6be113c..fb7d62d 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3076,6 +3076,23 @@ that affect more than one function.
>  This attribute should be used for debugging purposes only.  It is not
>  suitable in production code.
>  
> +@item prolog_pad
> +@cindex @code{prolog_pad} function attribute
> +@cindex function entry padded with NOPs
> +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.  Usually,
> +prolog padding is enabled globally using the @option{-fprolog-pad=N,M}
> +command-line switch, and disabled with attribute @code{prolog_pad (0)}
> +for functions that are part of the actual instrumentation framework.
> +This conveniently avoids an endless recursion.
> +Of course the @code{prolog_pad} function attribute can be used to

s/Of course the/The/

> +change the pad size to any desired value.  The two-value syntax is
> +the same as for the command-line switch @option{-fprolog-pad=N,M},
> +generating a NOP pad of size @var{N}, with the function entry point
> +@var{M} NOP instructions into the pad.  @var{M} defaults to 0
> +if omitted e.g. function entry point is the beginning of the pad.
> +
>  @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 69f0adb..d2e11bc 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11341,6 +11341,27 @@ 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 -fprolog-pad=@var{N},@var{M}
This needs to make it clear that M is optional.  Then below state that
if omitted, M defaults to zero.

> +@opindex fprolog-pad
> +Generate a pad of @var{N} NOPs right at the beginning
> +of each function, with the function entry point @var{M} NOPs into
> +the pad, which can be used to patch in any desired
> +instrumentation at run time, provided that the code segment
> +is writable.
> +For run-time identification, the starting addresses
> +of these pads, which correspond to their respective function entries
> +minus @var{M}, are additionally collected in the @code{__prolog_pads_loc}
> +section of the resulting binary.
> +
> +Note that value of @code{__attribute__ ((prolog_pad (N,M)))} takes
> +precedence over command-line option @option{-fprolog-pad=N,M}.
> +This can be used to increase the pad size or to remove the pad completely
> +on a single function.  If @code{N=0}, no pad location is recorded.
> +
> +The NOP pad is inserted at (and maybe before) the function entry address,
> +even before the prologue.  If @var{M} is omitted, it defaults to 0 e.g. the
> +pad starts exactly at the function entry point and not before.
> +

This is the only text most users will see, it also needs to make it
clear that the size of a NOP in a variable size instruction set is the
size of the smallest NOP on that machine.

>  @end table
>  
>  
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 4b62e05..eb29f71 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -4564,6 +4564,16 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
> +Generate an array of @var{pad_size} NOP instructions in the
> +output @var{file}.  If the target supports NOPs of different sizes,
> +the instruction with the smallest code size should be used, in order
> +to achieve the finest granularity for @var{pad_size}.  If @var{record_p}
> +is true, the current location should be recorded in the
> +@code{__prolog_pads_loc} section or a target-specific equivalent.
> +
> +@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 ea74d37..7b01026 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -3648,6 +3648,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_PROLOG_PAD
> +
>  @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 fccb8c6..d4955c0 100644
> --- a/gcc/lto/lto-lang.c
> +++ b/gcc/lto/lto-lang.c
> @@ -50,6 +50,7 @@ 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_prolog_pad_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 *);
> @@ -78,6 +79,8 @@ const struct attribute_spec lto_attribute_table[] =
>  			      handle_nonnull_attribute, false },
>    { "nothrow",                0, 0, true,  false, false,
>  			      handle_nothrow_attribute, false },
> +  { "prolog_pad",	      1, 2, true, false, false,
> +			      handle_prolog_pad_attribute, false },
>    { "returns_twice",          0, 0, true,  false, false,
>  			      handle_returns_twice_attribute, false },
>    { "sentinel",               0, 1, false, true, true,
> @@ -475,6 +478,14 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
>    return NULL_TREE;
>  }
>  
> +static tree
> +handle_prolog_pad_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 5f573a1..60d80aa 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2157,6 +2157,26 @@ common_handle_option (struct gcc_options *opts,
>          opts->x_flag_ipa_reference = false;
>        break;
>  
> +    case OPT_fprolog_pad_:
> +      {
> +	const char *comma = strchr (arg, ',');
> +	if (comma)
> +	  {
> +	    prolog_nop_pad_size = atoi (arg);
> +	    prolog_nop_pad_entry = atoi (comma + 1);
> +	  }
> +	else
> +	  {
> +	    prolog_nop_pad_size = atoi (arg);
> +	    prolog_nop_pad_entry = 0;
> +	  }

Where's the error checking?  If I write gibberish after the option name
then atoi will silently fail and return zero.  I'm not overly familiar
with the option handling code, but I'm sure we have routines to do the
heavy lifting here.


> +	if (prolog_nop_pad_size < 0
> +	    || prolog_nop_pad_entry < 0
> +	    || prolog_nop_pad_size < prolog_nop_pad_entry)
> +	  error ("invalid arguments for %<-fprolog_pad%>");
> +      }
> +      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 0443390..60e56cd 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -288,6 +288,12 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
>   void, (tree decl, int visibility),
>   default_assemble_visibility)
>  
> +DEFHOOK
> +(print_prolog_pad,
> + "Generate prologue pad",
> + void, (FILE *file, unsigned HOST_WIDE_INT pad_size, bool record_p),
> + default_print_prolog_pad)
> +
>  /* Output the assembler code for entry to a function.  */
>  DEFHOOK
>  (function_prologue,
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 2f2abd3..2ee5e38 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -1617,6 +1617,31 @@ default_compare_by_pieces_branch_ratio (machine_mode)
>    return 1;
>  }
>  
> +/* Write pad_size NOPs into the asm outfile before a function
> +   prologue.  If record_p is true, the location of the pad will be
> +   recorded in a special object section called "__prolog_pads_loc".
> +   This routine may be called twice per function to put NOPs before
> +   and after the function entry.  */
> +

The convention when referring to formal parameters in comments is to
write the name in upper case, so PAD_SIZE and RECORD_P.

> +void
> +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
> +			  bool record_p)
> +{
> +  if (record_p)
> +    fprintf (file, "1:");
> +
> +  unsigned i;
> +  for (i = 0; i < pad_size; ++i)
> +    fprintf (file, "\tnop\n");
> +
> +  if (record_p)
> +    {
> +      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
> +      fprintf (file, "\t.quad 1b\n");
> +      fprintf (file, "\t.previous\n");
> +    }
> +}

NO!  Almost everything in this function is wrong, it needs to be done
through suitable hooks that call into the machine back-ends that
understand assembly flavours supported.

What are you doing to do if you have an object format that does not
support named sections?

You're assuming a label syntax.

You're assuming that the assembler supports .previous

You're assuming that '@' is not a comment character (on ARM it is).

You're assuming we support relative labels.

You're assuming the size of the reference to a pad should be a '.quad'.

You're assuming that "nop" is a valid mnemonic in the assembler and
produces a minimally sized NOP.

There are hooks in the compiler to support all this sort of stuff; use them.

It perhaps wouldn't be so bad if you made it clear that this was
explicitly for ELF format (the comment issue still applies, as does the
use of .quad).

> +
>  bool
>  default_profile_before_prologue (void)
>  {
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index a5565f5..e302e8d 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -203,6 +203,7 @@ 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_prolog_pad (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/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> new file mode 100644
> index 0000000..2236aa8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fprolog-pad=3,1" } */
> +
> +void f1 (void) __attribute__((prolog_pad(2,1)));
> +void f2 (void) __attribute__((prolog_pad(3)));
> +int f3 (void);
> +
> +void
> +f1 (void)
> +{
> +  f2 ();
> +}
> +
> +void
> +f2 (void)
> +{
> +  f1 ();
> +}
> +
> +/* F3 should never have a NOP pad.  */
> +int
> +__attribute__((prolog_pad(0)))
> +__attribute__((noinline))
> +f3 (void)
> +{
> +  return 5;
> +}
> +
> +/* F4 should receive the command line default setting.  */
> +int
> +f4 (void)
> +{
> +  return 3*f3 ()+1;
> +}
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index c0f1a2d..649c4bd 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1595,8 +1595,10 @@ process_options (void)
>      }
>  
>   /* Do not use IPA optimizations for register allocation if profiler is active
> +    or prolog pads 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 || prolog_nop_pad_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 11a8ac4..6c40bce 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -1830,6 +1830,44 @@ assemble_start_function (tree decl, const char *fnname)
>    if (DECL_PRESERVE_P (decl))
>      targetm.asm_out.mark_decl_preserved (fnname);
>  
> +  unsigned HOST_WIDE_INT pad_size = prolog_nop_pad_size;
> +  unsigned HOST_WIDE_INT pad_entry = prolog_nop_pad_entry;
> +
> +  tree prolog_pad_attr
> +    = lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (decl));
> +  if (prolog_pad_attr)
> +    {
> +      tree pp_val = TREE_VALUE (prolog_pad_attr);
> +      tree prolog_pad_value1 = TREE_VALUE (pp_val);
> +
> +      if (tree_fits_uhwi_p (prolog_pad_value1))
> +	pad_size = tree_to_uhwi (prolog_pad_value1);
> +      else
> +	gcc_unreachable ();
> +
> +      pad_entry = 0;
> +      if (list_length (pp_val) > 1)
> +	{
> +	  tree prolog_pad_value2 = TREE_VALUE (TREE_CHAIN (pp_val));
> +
> +	  if (tree_fits_uhwi_p (prolog_pad_value2))
> +	    pad_entry = tree_to_uhwi (prolog_pad_value2);
> +	  else
> +	    gcc_unreachable ();
> +	}
> +    }
> +
> +  if (pad_entry > pad_size)
> +    {
> +      if (pad_size > 0)
> +	warning (OPT_Wattributes, "Prolog nop pad entry > size");
> +      pad_entry = 0;
> +    }
> +
> +  /* Emit the prolog padding before the entry label, if any.  */
> +  if (pad_entry > 0)
> +    targetm.asm_out.print_prolog_pad (asm_out_file, pad_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);
> @@ -1838,6 +1876,11 @@ 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 padding after the label.  Record it if we haven't done so yet.  */
> +  if (pad_size > pad_entry)
> +    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
> +				      (pad_entry == 0));
> +
>    if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
>      saw_no_split_stack = true;
>  }
> 

Re: [PATCH v5] add -fprolog-pad=N,M option

[Marek Polacek]

On Wed, Feb 15, 2017 at 11:01:16AM +0000, Richard Earnshaw (lists) wrote:
> On 13/01/17 12:19, Torsten Duwe wrote:
> > Changes since v4: hopefully addressed all of Sandra's requests
> > and suggestions concerning the documentation snippets, thanks
> > for the feedback.  If it still isn't clear, feel free to rephrase
> > -- I'm a programmer, not a technical writer.
> > 
> 
> Generally, I find 'pad' somewhat confusing.  It's OK for the option name
> itself, but more generally, we should be talking about either space or
> number of instructions according to context.
> 
> > 2017-01-13	Torsten Duwe	<duwe@suse.de> : 
> 
> Two spaces between date and name, two more between name and email, no
> colon at the end.
> 
> > 
> > 	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
> > 	   a handler for it.
> > 
> 
> 
> Don't leave blank lines between files mentioned here.  All lines should
> be indented by /exactly/ one tab; don't add extra indentation for
> continuation lines.  No space before colon.  Capital letter at start of
> sentences, full stop at the end of each one.  Which function, or data
> structure did you change (put the name in brackets before the colon)?
> Document each function or variable changed.
> 
> The above entry should read:
> 
> 	* c-family/c-attribs.c (c_common_attribute_table): Add entry
> 	for "prolog_pad".
> 	(handle_prolog_pad_attribute): New function.

Except that the c-family/ prefix shouldn't be there.

	Marek

Re: [PATCH v5] add -fprolog-pad=N,M option

[Richard Earnshaw (lists)]

On 15/02/17 11:12, Marek Polacek wrote:
> On Wed, Feb 15, 2017 at 11:01:16AM +0000, Richard Earnshaw (lists) wrote:
>> On 13/01/17 12:19, Torsten Duwe wrote:
>>> Changes since v4: hopefully addressed all of Sandra's requests
>>> and suggestions concerning the documentation snippets, thanks
>>> for the feedback.  If it still isn't clear, feel free to rephrase
>>> -- I'm a programmer, not a technical writer.
>>>
>>
>> Generally, I find 'pad' somewhat confusing.  It's OK for the option name
>> itself, but more generally, we should be talking about either space or
>> number of instructions according to context.
>>
>>> 2017-01-13	Torsten Duwe	<duwe@suse.de> : 
>>
>> Two spaces between date and name, two more between name and email, no
>> colon at the end.
>>
>>>
>>> 	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
>>> 	   a handler for it.
>>>
>>
>>
>> Don't leave blank lines between files mentioned here.  All lines should
>> be indented by /exactly/ one tab; don't add extra indentation for
>> continuation lines.  No space before colon.  Capital letter at start of
>> sentences, full stop at the end of each one.  Which function, or data
>> structure did you change (put the name in brackets before the colon)?
>> Document each function or variable changed.
>>
>> The above entry should read:
>>
>> 	* c-family/c-attribs.c (c_common_attribute_table): Add entry
>> 	for "prolog_pad".
>> 	(handle_prolog_pad_attribute): New function.
> 
> Except that the c-family/ prefix shouldn't be there.
> 

Ah, I'd missed that it has its own ChangeLog file and thus needs to be
recorded separately.  Ergo, the LTO entry can't use Likewise since the
context for that will not be immediately above that entry...

R.

> 	Marek
> 

[PATCH v6] add -fprolog-pad=N,M option

[Torsten Duwe]

Hi,

Thanks for all the feedback. Hopefully it's all incorporated now.
I will reply to you individually on the specific topics, but here is
the new v6 for you to rip apart ;-)

Changes since v5:

* ChangeLogs split, reshuffled, reformatted.

* cmdline option parsing again with integral_argument ()

* Documentation has less "pad"s

* completely reworked default_print_prolog_pad ()
  -- never liked the old version either.

	Torsten



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

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

gcc/lto/ChangeLog
2017-02-17  Torsten Duwe  <duwe@suse.de>

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

gcc/ChangeLog
2017-02-17  Torsten Duwe  <duwe@suse.de>

	* common.opt: Introduce -fprolog_pad command line option,
	and its variables prolog_nop_pad_size and prolog_nop_pad_entry.
	* opts.c (common_handle_option): Add -fprolog_pad_ case,
	including a two-value parser.
	* target.def (print_prolog_pad): New target hook.
	* targhooks.h (default_print_prolog_pad): New function.
	* targhooks.c (default_print_prolog_pad): Likewise.
	* toplev.c (process_options): Switch off IPA-RA if
	prolog pads are being generated.
	* varasm.c (assemble_start_function): Look at the prolog-pad command
	line switch and current function attributes and maybe generate NOP
	instructions by calling the print_prolog_pad hook.
	* doc/extend.texi: Document prolog_pad attribute.
	* doc/invoke.texi: Document -fprolog_pad command line option.
	* doc/tm.texi.in (TARGET_ASM_PRINT_PROLOG_PAD): New target hook.
	* doc/tm.texi: Likewise.

gcc/testsuite/ChangeLog
2017-02-17  Torsten Duwe  <duwe@suse.de>

	* c-c++-common/attribute-prolog_pad-1.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index ce7fcaa..9f0f580 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -139,6 +139,7 @@ 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_prolog_pad_attribute (tree *, tree, tree, int, bool *);
 
 /* Table of machine-independent attributes common to all C-like languages.
 
@@ -345,6 +346,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			      handle_bnd_instrument, false },
   { "fallthrough",	      0, 0, false, false, false,
 			      handle_fallthrough_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { NULL,                     0, 0, false, false, false, NULL, false }
 };
 
@@ -3173,3 +3176,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int,
   *no_add_attrs = true;
   return NULL_TREE;
 }
+
+static tree
+handle_prolog_pad_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 ad6baa3..02993b1 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 before each function prologue by default
+Variable
+HOST_WIDE_INT prolog_nop_pad_size
+
+; And how far the asm entry point is into this pad
+Variable
+HOST_WIDE_INT prolog_nop_pad_entry
 
 ; Balance between GNAT encodings and standard DWARF to emit.
 Variable
@@ -2022,6 +2029,10 @@ fprofile-reorder-functions
 Common Report Var(flag_profile_reorder_functions)
 Enable function reordering that improves code placement.
 
+fprolog-pad=
+Common Joined Optimization
+Insert NOP instructions before each function prologue.
+
 frandom-seed
 Common Var(common_deferred_options) Defer
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3d1546a..ef7e985 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3076,6 +3076,23 @@ that affect more than one function.
 This attribute should be used for debugging purposes only.  It is not
 suitable in production code.
 
+@item prolog_pad
+@cindex @code{prolog_pad} 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.  Usually,
+prolog padding is enabled globally using the @option{-fprolog-pad=N,M}
+command-line switch, and disabled with attribute @code{prolog_pad (0)}
+for functions that are part of the actual instrumentation framework.
+This conveniently avoids an endless recursion.
+The @code{prolog_pad} function attribute can be used to
+change the pad size to any desired value.  The two-value syntax is
+the same as for the command-line switch @option{-fprolog-pad=N,M},
+generating a NOP pad of size @var{N}, with the function entry point
+@var{M} NOP instructions into the pad.  @var{M} defaults to 0
+if omitted e.g. function entry point is before the first NOP.
+
 @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 56ca53f..75a7e2c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11370,6 +11370,31 @@ 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 -fprolog-pad=@var{N}[,@var{M}]
+@opindex fprolog-pad
+Generate a pad of @var{N} NOPs right at the beginning
+of each function, with the function entry point @var{M} NOPs into
+the pad.  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 only controllable indirectly via
+the number of NOPs, so implementers are advised to use the smallest
+NOP instruction available for the current CPU mode should there be a
+choice, in order to achieve the finest granularity.
+For run-time identification, the starting addresses
+of these pads, which correspond to their respective function entries
+minus @var{M}, are additionally collected in the @code{__prolog_pads_loc}
+section of the resulting binary.
+
+Note that the value of @code{__attribute__ ((prolog_pad (N,M)))} takes
+precedence over command-line option @option{-fprolog-pad=N,M}.
+This can be used to increase the pad 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) the function entry
+address, even before the prologue.
+
 @end table
 
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 348fd68..5155d10 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4566,6 +4566,10 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
+Generate prologue pad
+@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 6cde83c..b1d9d99 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_PROLOG_PAD
+
 @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 ca8945e..9143328 100644
--- a/gcc/lto/lto-lang.c
+++ b/gcc/lto/lto-lang.c
@@ -48,6 +48,7 @@ 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_prolog_pad_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 +77,8 @@ const struct attribute_spec lto_attribute_table[] =
 			      handle_nonnull_attribute, false },
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
+  { "prolog_pad",	      1, 2, true, false, false,
+			      handle_prolog_pad_attribute, false },
   { "returns_twice",          0, 0, true,  false, false,
 			      handle_returns_twice_attribute, false },
   { "sentinel",               0, 1, false, true, true,
@@ -473,6 +476,13 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name),
   return NULL_TREE;
 }
 
+static tree
+handle_prolog_pad_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 b38e9b4..10f751f 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2159,6 +2159,29 @@ common_handle_option (struct gcc_options *opts,
         opts->x_flag_ipa_reference = false;
       break;
 
+    case OPT_fprolog_pad_:
+      {
+	char *pad_arg = xstrdup (arg);
+	char *comma = strchr (pad_arg, ',');
+	if (comma)
+	  {
+	    *comma = '\0';
+	    prolog_nop_pad_size = integral_argument (pad_arg);
+	    prolog_nop_pad_entry = integral_argument (comma + 1);
+	  }
+	else
+	  {
+	    prolog_nop_pad_size = integral_argument (pad_arg);
+	    prolog_nop_pad_entry = 0;
+	  }
+	if (prolog_nop_pad_size < 0
+	    || prolog_nop_pad_entry < 0
+	    || prolog_nop_pad_size < prolog_nop_pad_entry)
+	  error ("invalid arguments for %<-fprolog_pad%>");
+	free (pad_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 43600ae..bdc47b4 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -288,6 +288,12 @@ hidden, protected or internal visibility as specified by @var{visibility}.",
  void, (tree decl, int visibility),
  default_assemble_visibility)
 
+DEFHOOK
+(print_prolog_pad,
+ "Generate prologue pad",
+ void, (FILE *file, unsigned HOST_WIDE_INT pad_size, bool record_p),
+ default_print_prolog_pad)
+
 /* Output the assembler code for entry to a function.  */
 DEFHOOK
 (function_prologue,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 1cdec06..6729e6c 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1609,6 +1609,52 @@ default_compare_by_pieces_branch_ratio (machine_mode)
   return 1;
 }
 
+/* Write PAD_SIZE NOPs into the asm outfile FILE before a function
+   prologue.  If RECORD_P is true, the location of the pad will be
+   recorded in a special object section called "__prolog_pads_loc".
+   This routine may be called twice per function to put NOPs before
+   and after the function entry.  */
+
+void
+default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
+			  bool record_p)
+{
+  static const char *nop_templ = 0;
+
+  /* We use the template alone, relying on the (currently sane) assumption
+     that the NOP template does not have variable operands.  */
+  if (!nop_templ)
+    {
+      int code_num;
+      rtx_insn *my_nop = make_insn_raw (gen_nop ());
+
+      code_num = recog_memoized (my_nop);
+      nop_templ = get_insn_template (code_num, my_nop);
+    }
+
+  if (record_p)
+    {
+      char buf[256];
+      static int pad_number;
+      section *previous_section = in_section;
+
+      pad_number++;
+      ASM_GENERATE_INTERNAL_LABEL (buf, "LPPAD", pad_number);
+
+      switch_to_section (get_section ("__prolog_pads_loc", 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);
+    }
+
+  unsigned i;
+  for (i = 0; i < pad_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 a5565f5..e302e8d 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -203,6 +203,7 @@ 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_prolog_pad (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/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
new file mode 100644
index 0000000..2236aa8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-fprolog-pad=3,1" } */
+
+void f1 (void) __attribute__((prolog_pad(2,1)));
+void f2 (void) __attribute__((prolog_pad(3)));
+int f3 (void);
+
+void
+f1 (void)
+{
+  f2 ();
+}
+
+void
+f2 (void)
+{
+  f1 ();
+}
+
+/* F3 should never have a NOP pad.  */
+int
+__attribute__((prolog_pad(0)))
+__attribute__((noinline))
+f3 (void)
+{
+  return 5;
+}
+
+/* F4 should receive the command line default setting.  */
+int
+f4 (void)
+{
+  return 3*f3 ()+1;
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index beb581a..3afda4a 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1596,8 +1596,10 @@ process_options (void)
     }
 
  /* Do not use IPA optimizations for register allocation if profiler is active
+    or prolog pads 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 || prolog_nop_pad_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 11a8ac4..84c739a 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1830,6 +1830,44 @@ assemble_start_function (tree decl, const char *fnname)
   if (DECL_PRESERVE_P (decl))
     targetm.asm_out.mark_decl_preserved (fnname);
 
+  unsigned HOST_WIDE_INT pad_size = prolog_nop_pad_size;
+  unsigned HOST_WIDE_INT pad_entry = prolog_nop_pad_entry;
+
+  tree prolog_pad_attr
+    = lookup_attribute ("prolog_pad", DECL_ATTRIBUTES (decl));
+  if (prolog_pad_attr)
+    {
+      tree pp_val = TREE_VALUE (prolog_pad_attr);
+      tree prolog_pad_value1 = TREE_VALUE (pp_val);
+
+      if (tree_fits_uhwi_p (prolog_pad_value1))
+	pad_size = tree_to_uhwi (prolog_pad_value1);
+      else
+	gcc_unreachable ();
+
+      pad_entry = 0;
+      if (list_length (pp_val) > 1)
+	{
+	  tree prolog_pad_value2 = TREE_VALUE (TREE_CHAIN (pp_val));
+
+	  if (tree_fits_uhwi_p (prolog_pad_value2))
+	    pad_entry = tree_to_uhwi (prolog_pad_value2);
+	  else
+	    gcc_unreachable ();
+	}
+    }
+
+  if (pad_entry > pad_size)
+    {
+      if (pad_size > 0)
+	warning (OPT_Wattributes, "Prolog nop pad entry > size");
+      pad_entry = 0;
+    }
+
+  /* Emit the prolog padding before the entry label, if any.  */
+  if (pad_entry > 0)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_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);
@@ -1838,6 +1876,11 @@ 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 padding after the label.  Record it if we haven't done so yet.  */
+  if (pad_size > pad_entry)
+    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
+				      pad_entry == 0);
+
   if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
     saw_no_split_stack = true;
 }

Re: [PATCH v5] add -fprolog-pad=N,M option

[Torsten Duwe]

On Wed, Feb 15, 2017 at 11:01:16AM +0000, Richard Earnshaw (lists) wrote:
> On 13/01/17 12:19, Torsten Duwe wrote:
> 
> > +++ b/gcc/doc/invoke.texi
> > @@ -11341,6 +11341,27 @@ 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 -fprolog-pad=@var{N},@var{M}
> This needs to make it clear that M is optional.  Then below state that
> if omitted, M defaults to zero.

It was mentioned, further down in the paragraph. I moved it up.

> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -2157,6 +2157,26 @@ common_handle_option (struct gcc_options *opts,
> >          opts->x_flag_ipa_reference = false;
> >        break;
> >  
> > +    case OPT_fprolog_pad_:
> > +      {
> > +	const char *comma = strchr (arg, ',');
> > +	if (comma)
> > +	  {
> > +	    prolog_nop_pad_size = atoi (arg);
> > +	    prolog_nop_pad_entry = atoi (comma + 1);
> > +	  }
> > +	else
> > +	  {
> > +	    prolog_nop_pad_size = atoi (arg);
> > +	    prolog_nop_pad_entry = 0;
> > +	  }
> 
> Where's the error checking?  If I write gibberish after the option name
> then atoi will silently fail and return zero.  I'm not overly familiar
> with the option handling code, but I'm sure we have routines to do the
> heavy lifting here.

Yes, I had already found integral_argument, but that's unsuitable for a
comma separated list, and arg is const so I could' punch a \0 there.
Using atoi was just lazy, admittedly.

> > +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
> > +			  bool record_p)
> > +{
> > +  if (record_p)
> > +    fprintf (file, "1:");
> > +
> > +  unsigned i;
> > +  for (i = 0; i < pad_size; ++i)
> > +    fprintf (file, "\tnop\n");
> > +
> > +  if (record_p)
> > +    {
> > +      fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
> > +      fprintf (file, "\t.quad 1b\n");
> > +      fprintf (file, "\t.previous\n");
> > +    }
> > +}
> 
> NO!  Almost everything in this function is wrong, it needs to be done
> through suitable hooks that call into the machine back-ends that
> understand assembly flavours supported.

That was already mentioned in a previous version. That code assumes GAS+ELF.
It was the quick and dirty solution to get a working prototype.

	Torsten

Re: [PATCH v5] add -fprolog-pad=N,M option

[Torsten Duwe]

On Wed, Feb 08, 2017 at 12:48:56PM +0100, Jakub Jelinek wrote:
> 
> First of all, GCC is in stage4 and this isn't a fix for a regression, so it
> is not acceptable at this point.  Next stage1 will start in April or so.

I had gotten the impression there were sort of branches in the SCM; this
of course should go to HEAD / trunk / master or whatever it's called.

> The length of nop varies greatly across different architectures,
> some architectures have different spelling for it (e.g.
> ia64, s390{,x} use nop 0 and mmix uses swym 0), some architectures have
> tons of different nops with different sizes (see e.g. x86_64/i686), for some
> longer sizes it is often beneficial to emit a jump around the rest of nops
> instead of just tons of nops.
> 
> Even if it is counted always in nops and only nops are emitted (not really
> efficient, and I guess kernel cares about efficiency), the above is

Yes, efficiency is a goal, but not unchallenged. Besides, out of order micro-
architectures hardly suffer from NOPs inserted other than their occupation of
cache space and memory bandwidth. The worst impact I found on some in-order
aarch64 CPUs was a penalty of 1 clock cycle for 2 NOPs.

Note that this is a "you asked for it -- you get it" feature. Only if you
explicitly request those NOPs they will be inserted; normal operation is
unaffected.

> definitely not acceptable for a generic hook implementation, it contains too
> many ELF specific details as well as 64-bit target specific details etc.
> For the label, you should use something like:
>   static int ppad_no;
>   char tmp_label[...];
>   ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LPPAD", ppad_no);
>   ppad_no++;
>   ASM_OUTPUT_LABEL (file, tmp_label);
> the "a",@progbits is not generic enough, you want
>   switch_to_section (get_section ("__prolog_pads_loc", 0, NULL));
> or so.  .previous doesn't work on many targets, you need to actuall switch
> to the previously current section.  .quad is not supported by many
> assemblers, you want assemble_integer, with the appropriate size (say
> derived from size of pointer), decide whether in that section everything is
> aligned or unaligned, emit aligning directive if the former.

Thanks a lot for this blueprint! It saved me half of the work for the rewrite!

> There are many other ways in which the ABI can check, e.g.
> see how i386.c (ix86_function_regparm) can change ABI if
>           cgraph_local_info *i = &target->local;
>           if (i && i->local && i->can_change_signature)
> So you'd e.g. need to make sure that functions you emit the padding for
> can't change signature.  Bet you won't be able to handle e.g. IPA cp or many
> other IPA optimizations that change signature too, while the function from
> the source may still be around, perhaps nothing will call it because the
> callers have been changed to call its clone with different arguments.
> Or say IPA-ICF, if you want to live-patch only some function and not another
> function which happens to have identical bogy in the end.

Indeed. Live patching needs to take more care. But as I wrote, it is only
_my_ current use case; the feature itself should be usable more widely.
I wouldn't want to restrict future users on what they can optimise and what
they can't. Maybe someone wants to measure exactly these effects?

The point is: whatever instructions are to replace those NOPs, they will
very likely need a register or two. With IPA-RA, all bets are off. Without,
you can assume the caller-save regs, the scratch regs and some intra-procedure
regs to be available. Please let's leave this all to the framework
implementers. The same goes for nops before or after the entry point. This
is only a _mechanism_.

	Torsten

Re: [PATCH v6] add -fprolog-pad=N,M option

[Sandra Loosemore]

On 02/17/2017 09:57 AM, Torsten Duwe wrote:

> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 3d1546a..ef7e985 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3076,6 +3076,23 @@ that affect more than one function.
>   This attribute should be used for debugging purposes only.  It is not
>   suitable in production code.
>
> +@item prolog_pad
> +@cindex @code{prolog_pad} function attribute

I'm only a documentation maintainer so this is out of my area of 
responsibility, but I really wish we could rename the attribute and 
command-line option.  Per

per https://gcc.gnu.org/codingconventions.html#Spelling

the correct spelling is "prologue".

> +@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.  Usually,
> +prolog padding is enabled globally using the @option{-fprolog-pad=N,M}

definitely s/prolog/prologue/ in the running text here.

> +command-line switch, and disabled with attribute @code{prolog_pad (0)}
> +for functions that are part of the actual instrumentation framework.
> +This conveniently avoids an endless recursion.
> +The @code{prolog_pad} function attribute can be used to
> +change the pad size to any desired value.  The two-value syntax is
> +the same as for the command-line switch @option{-fprolog-pad=N,M},

Add a cross-reference here.

> +generating a NOP pad of size @var{N}, with the function entry point

Sizes are usually expressed in bytes.  I think some other unit is 
intended here, though, so I'd avoid "size" and use some other way to 
describe it.  Maybe "generating a pad of @var{N} NOP instructions".

> +@var{M} NOP instructions into the pad.  @var{M} defaults to 0
> +if omitted e.g. function entry point is before the first NOP.
> +
>   @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 56ca53f..75a7e2c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11370,6 +11370,31 @@ 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 -fprolog-pad=@var{N}[,@var{M}]
> +@opindex fprolog-pad
> +Generate a pad of @var{N} NOPs right at the beginning
> +of each function, with the function entry point @var{M} NOPs into
> +the pad.  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 only controllable indirectly via
> +the number of NOPs, so implementers are advised to use the smallest
> +NOP instruction available for the current CPU mode should there be a
> +choice, in order to achieve the finest granularity.

The audience of the GCC user manual is users, not implementers.  If this 
is really just "advice" on what the option should do in the presence of 
multiple instruction sizes, and not a firm requirement, then rewrite as 
something like:

The amount of space reserved is expressed as the number of NOP 
instructions to insert. On targets that have multiple instruction sizes, 
typically the smallest NOP instruction available for the current CPU 
mode is used to achieve the finest granularity.

...except that I don't think "CPU mode" is really what you intend here. 
  E.g. on nios2, support for 16-bit instructions is a code generation 
option (-mcdx) rather than a -mcpu= or -march= option, and there is 
certainly no runtime processor mode selection involved.

If this is really a firm requirement, I think the burden is on you to 
identify all backends that have multiple NOP sizes for which the default 
hook implementation won't give the required behavior, and either provide 
an appropriate hook or work with the backend maintainers to develop one.

I'd put a paragraph break here, before:

> +For run-time identification, the starting addresses
> +of these pads, which correspond to their respective function entries
> +minus @var{M}, are additionally collected in the @code{__prolog_pads_loc}
> +section of the resulting binary.
> +
> +Note that the value of @code{__attribute__ ((prolog_pad (N,M)))} takes
> +precedence over command-line option @option{-fprolog-pad=N,M}.

@var{N} and @var{M} in both places, please.  And add a cross-reference.

> +This can be used to increase the pad size or to remove it completely
> +on a single function.  If @code{N=0}, no pad location is recorded.

That's sloppy markup.  How about

If @var{N} is zero, ....

> +The NOP instructions are inserted at (and maybe before) the function entry
> +address, even before the prologue.
> +
>   @end table
>
>
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 348fd68..5155d10 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -4566,6 +4566,10 @@ 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_PROLOG_PAD (FILE *@var{file}, unsigned HOST_WIDE_INT @var{pad_size}, bool @var{record_p})
> +Generate prologue pad

Sigh again.  Every other prologue-related target hook uses the PROLOGUE 
spelling.  :-S

Missing punctuation at the end of the sentence, and there's not nearly 
enough information about what this hook should do.

Also, I suggest moving this down in the section instead of listing it 
first, because of the usual principle of giving the most important 
information first.

> +@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,

-Sandra

Poll for option name (Was: [PATCH v6] add -fprolog-pad=N,M option)

[Torsten Duwe]

On Fri, Feb 17, 2017 at 11:30:29PM -0700, Sandra Loosemore wrote:
> >
> >+@item prolog_pad
> >+@cindex @code{prolog_pad} function attribute
> 
> I'm only a documentation maintainer so this is out of my area of
> responsibility, but I really wish we could rename the attribute and
> command-line option.  Per
> 
> per https://gcc.gnu.org/codingconventions.html#Spelling
> 
> the correct spelling is "prologue".
> 
> >+@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.  Usually,
> >+prolog padding is enabled globally using the @option{-fprolog-pad=N,M}
> 
> definitely s/prolog/prologue/ in the running text here.

Well, you're definitely right in both cases.

About 400 occurrences of "prolog" in the source without ChangeLogs,
mainly in gcc/tree-vect-loop-manip.c and in libgcc/config/libbid/bid_conf.h;
about 3000 lines with "prologue". However there is a "-mprolog-function"
switch. One might call this a broken window. I don't want to contribute
to that.

However, writing some more documentation and being asked for clarity,
I found it more depicting to talk about the function entry point than
about the prologue. Also, this is about generic instrumentation, and it
surely involves NOPs.

So, hereby I'd like to start a small poll for a good name for this feature.
Anyone with a better idea please speak up now. Otherwise I'll just
s/prolog/prologue/g.

> The amount of space reserved is expressed as the number of NOP instructions
> to insert. On targets that have multiple instruction sizes, typically the
> smallest NOP instruction available for the current CPU mode is used to
> achieve the finest granularity.

I've made another improvement which makes the code even more robust now.
+DEF_TARGET_INSN (nop, (void))
In gcc/target-insns.def. This way I can easily check whether there is a
(define_insn "nop" ...) in the target md. Currently, all CPUs have it, but
who knows.

This will also be the default instruction used (It can be overridden
in the terget hook), so that rule has changed.

So, before the next version, any clever name suggestions?

	Torsten

Re: Poll for option name (Was: [PATCH v6] add -fprolog-pad=N,M option)

[Richard Earnshaw (lists)]

On 01/03/17 11:26, Torsten Duwe wrote:
> On Fri, Feb 17, 2017 at 11:30:29PM -0700, Sandra Loosemore wrote:
>>>
>>> +@item prolog_pad
>>> +@cindex @code{prolog_pad} function attribute
>>
>> I'm only a documentation maintainer so this is out of my area of
>> responsibility, but I really wish we could rename the attribute and
>> command-line option.  Per
>>
>> per https://gcc.gnu.org/codingconventions.html#Spelling
>>
>> the correct spelling is "prologue".
>>
>>> +@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.  Usually,
>>> +prolog padding is enabled globally using the @option{-fprolog-pad=N,M}
>>
>> definitely s/prolog/prologue/ in the running text here.
> 
> Well, you're definitely right in both cases.
> 
> About 400 occurrences of "prolog" in the source without ChangeLogs,
> mainly in gcc/tree-vect-loop-manip.c and in libgcc/config/libbid/bid_conf.h;
> about 3000 lines with "prologue". However there is a "-mprolog-function"
> switch. One might call this a broken window. I don't want to contribute
> to that.
> 
> However, writing some more documentation and being asked for clarity,
> I found it more depicting to talk about the function entry point than
> about the prologue. Also, this is about generic instrumentation, and it
> surely involves NOPs.
> 
> So, hereby I'd like to start a small poll for a good name for this feature.
> Anyone with a better idea please speak up now. Otherwise I'll just
> s/prolog/prologue/g.

Hmm, I'd prefer the bike shed to be green :-)

How about --fpatchable-function-entry=<size-spec>?

> 
>> The amount of space reserved is expressed as the number of NOP instructions
>> to insert. On targets that have multiple instruction sizes, typically the
>> smallest NOP instruction available for the current CPU mode is used to
>> achieve the finest granularity.
> 
> I've made another improvement which makes the code even more robust now.
> +DEF_TARGET_INSN (nop, (void))
> In gcc/target-insns.def. This way I can easily check whether there is a
> (define_insn "nop" ...) in the target md. Currently, all CPUs have it, but
> who knows.

The mid-end already has direct calls to gen_nop with no guards on the
pattern existing,  So the compiler won't build without a NOP pattern.

> 
> This will also be the default instruction used (It can be overridden
> in the terget hook), so that rule has changed.
> 
> So, before the next version, any clever name suggestions?
> 
> 	Torsten
> 

R.

Re: Poll for option name (Was: [PATCH v6] add -fprolog-pad=N,M option)

[Torsten Duwe]

On Wed, Mar 01, 2017 at 11:34:37AM +0000, Richard Earnshaw (lists) wrote:
> On 01/03/17 11:26, Torsten Duwe wrote:
> > 
> > However, writing some more documentation and being asked for clarity,
> > I found it more depicting to talk about the function entry point than
> > about the prologue. Also, this is about generic instrumentation, and it
> > surely involves NOPs.
> > 
> > So, hereby I'd like to start a small poll for a good name for this feature.
> > Anyone with a better idea please speak up now. Otherwise I'll just
> > s/prolog/prologue/g.
> 
> Hmm, I'd prefer the bike shed to be green :-)
> 
> How about --fpatchable-function-entry=<size-spec>?
> 
IMHO qualifies as "better". And green is best anyway :-]

> > I've made another improvement which makes the code even more robust now.
> > +DEF_TARGET_INSN (nop, (void))
> > In gcc/target-insns.def. This way I can easily check whether there is a
> > (define_insn "nop" ...) in the target md. Currently, all CPUs have it, but
> > who knows.
> 
> The mid-end already has direct calls to gen_nop with no guards on the
> pattern existing,  So the compiler won't build without a NOP pattern.

Richard told me "don't do that", and we found the DEF_TARGET_INSN. So far
I can see gen_nop only in target specifics and in cfgrtl.c -- admittedly
I don't know what that does.

So the v6 code is basically OK?

Names better than -fpatchable-function-entry anyone?

	Torsten

Re: Poll for option name (Was: [PATCH v6] add -fprolog-pad=N,M option)

[Richard Earnshaw (lists)]

On 01/03/17 13:32, Torsten Duwe wrote:
> On Wed, Mar 01, 2017 at 11:34:37AM +0000, Richard Earnshaw (lists) wrote:
>> On 01/03/17 11:26, Torsten Duwe wrote:
>>>
>>> However, writing some more documentation and being asked for clarity,
>>> I found it more depicting to talk about the function entry point than
>>> about the prologue. Also, this is about generic instrumentation, and it
>>> surely involves NOPs.
>>>
>>> So, hereby I'd like to start a small poll for a good name for this feature.
>>> Anyone with a better idea please speak up now. Otherwise I'll just
>>> s/prolog/prologue/g.
>>
>> Hmm, I'd prefer the bike shed to be green :-)
>>
>> How about --fpatchable-function-entry=<size-spec>?
>>
> IMHO qualifies as "better". And green is best anyway :-]
> 
>>> I've made another improvement which makes the code even more robust now.
>>> +DEF_TARGET_INSN (nop, (void))
>>> In gcc/target-insns.def. This way I can easily check whether there is a
>>> (define_insn "nop" ...) in the target md. Currently, all CPUs have it, but
>>> who knows.
>>
>> The mid-end already has direct calls to gen_nop with no guards on the
>> pattern existing,  So the compiler won't build without a NOP pattern.
> 
> Richard told me "don't do that", and we found the DEF_TARGET_INSN. So far
> I can see gen_nop only in target specifics and in cfgrtl.c -- admittedly
> I don't know what that does.
> 
> So the v6 code is basically OK?
> 
I haven't reviewed it yet.  I'm not really planning to spend any more
time on this until stage1 re-opens.

R.

> Names better than -fpatchable-function-entry anyone?
> 
> 	Torsten
>