This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Patch to enable efficient function level instrumentation (issue5416043)
- From: harshit at google dot com (Harshit Chopra)
- To: reply at codereview dot appspotmail dot com, gcc-patches at gcc dot gnu dot org
- Date: Thu, 17 Nov 2011 15:38:39 -0800 (PST)
- Subject: Patch to enable efficient function level instrumentation (issue5416043)
>From f63ec02c0a720174489fe450b3cc43eb00fd4bdd Mon Sep 17 00:00:00 2001
From: Harshit Chopra <harshit@google.com>
Date: Thu, 3 Nov 2011 17:29:23 -0700
Subject: [PATCH] Mechanism to provide efficient function instrumentation (x86-64)
Summary: This patch aims at providing an efficient way to instrument a binary at function level for x86-64 architecture. Existing mechanisms (like -finstrument-functions) are expensive, especially when not instrumenting, which calls for the need of separate binaries for deployment and debugging for long running systems. This patch aims to remove this need of two separate binaries.
Detail: This patch adds a mechanism to insert a sequence of Nop bytes at function entry and at function exits, which are used to instrument a binary at function level by manipulating these nops at runtime. Emission of these nop bytes is controlled by the flag -mpatch-functions-for-instrumentation and other related flags. This patch only adds support for binaries for x86-64 architecture.
For each function in the binary, 11-bytes of nops are added at the entry and 10-bytes of nops at exit points (just after the return). For functions which make a tail call to another function, 11-bytes of nops are added just before the tail call jump. These nops can be patched at runtime to call appropriate instrumentation routines for function entry and exit.
The format of 11 bytes of nops (at function prologue and before tail call jump) is:
L0: jmp L1
XX
.quad L2
.section <function_patch_section_name>
L2: .quad L0
.text
L1: ... (function code)
The section 'function_patch_section_name' is used to store a back pointer to these nops. If the nops belong to function prologue, the name of this section is "_function_patch_prologue", or else if the nops belong to function exit, the name is "_function_patch_epilogue".
The 'XX' bytes can be anything (0x00-0xff) since they are dead bytes and act as fillers.
Similarly at function exit (just after ret instruction), the format of the 10-bytes of nops is:
...
L0: ret
XX
XX
.quad L1
.section _function_patch_epilogue
L1: .quad L0
In order to handle functions belonging to COMDAT group, the name of the function patch section is "<function_patch_section_name>.<function_decl_section>". Such sections are later renamed to "<function_patch_section_name>" just before emitting the assembly by the routine ix86_elf_asm_named_section().
Conceptually, these nops can be replaced by the following instructions at runtime:
mov <func_id>, %r10d
call InstrumentationFunction
The 'mov' being a 6 byte instruction and the call being a 5-byte instruction, a total of 11-bytes are needed for patching. Hence, the need for 11-bytes of nops. 10-bytes of nops are needed after the 'ret' instruction since the return is also patched and the call replaced by a jump to InstrumentationFunction (similar to a tail call).
To control emission of these nops globally and on a per-function basis (somewhat), the following flags are provided:
* -mpatch-functions-for-instrumentation: Master flag to control emission of these nops. The rest of the flags below have no effect if this flag is not provided.
* -mpatch-functions-min-instructions: If provided, only those functions having number of instructions greater than this flag value will have these nops at prologue and exit. Default is 200.
* -mpatch-functions-ignore-loops: To ignore loops when deciding whether to have these nops for a function. By default, functions having loops always include these nops.
* -mno-patch-functions-main-always: The 'main' function always has these nops to have the ability to rely on a function to call initialize instrumentation code. This flag treats main function as any other function.
Some issues with this patch:
* This patch prohibits garbage collection of dead functions at link time since there are pointers from the sections "_function_patch_prologue" and "_function_patch_epilogue" to the dead function.
* The existence of random data bytes in the dead code part of these nops in the middle of text section confuses disassembling tools like objdump.
Suggestions/comments for this patch are welcome. Also, suggestions on dealing with the above issues will be really helpful.
Testing done: Wrote tests to test code generation for the different cases and then ran 'make -k check-gcc'
Patch to be applied to google/main and trunk.
---
gcc/config/i386/i386-protos.h | 4 +
gcc/config/i386/i386.c | 199 +++++++++++++++++++++
gcc/config/i386/i386.md | 26 +++-
gcc/config/i386/i386.opt | 16 ++
gcc/testsuite/gcc.target/i386/patch-functions-1.c | 14 ++
gcc/testsuite/gcc.target/i386/patch-functions-2.c | 12 ++
gcc/testsuite/gcc.target/i386/patch-functions-3.c | 12 ++
gcc/testsuite/gcc.target/i386/patch-functions-4.c | 13 ++
gcc/testsuite/gcc.target/i386/patch-functions-5.c | 13 ++
gcc/testsuite/gcc.target/i386/patch-functions-6.c | 13 ++
gcc/testsuite/gcc.target/i386/patch-functions-7.c | 13 ++
11 files changed, 333 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-5.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-6.c
create mode 100644 gcc/testsuite/gcc.target/i386/patch-functions-7.c
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 900d1c5..f857bfd 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -29,6 +29,10 @@ extern bool ix86_handle_option (struct gcc_options *opts,
extern bool ix86_target_stack_probe (void);
extern bool ix86_can_use_return_insn_p (void);
extern void ix86_setup_frame_addresses (void);
+extern bool ix86_output_function_nops_prologue_epilogue (FILE *,
+ const char *,
+ const char *,
+ unsigned int);
extern HOST_WIDE_INT ix86_initial_elimination_offset (int, int);
extern void ix86_expand_prologue (void);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 38fea4e..e572040 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -60,6 +60,7 @@ along with GCC; see the file COPYING3. If not see
#include "fibheap.h"
#include "opts.h"
#include "diagnostic.h"
+#include "cfgloop.h"
enum upper_128bits_state
{
@@ -10792,6 +10793,190 @@ ix86_expand_epilogue (int style)
m->fs = frame_state_save;
}
+/* True if the current function should be patched with nops at prologue and
+ returns. */
+static bool patch_current_function_p = false;
+
+/* Return true if we patch the current function. */
+static bool
+check_should_patch_current_function (void)
+{
+ int num_insns = 0;
+ rtx insn;
+ const char* func_name = NULL;
+ struct loops loops;
+ int num_loops = 0;
+
+ /* Patch the function if it has at least a loop. */
+ if (!patch_functions_ignore_loops)
+ {
+ if (DECL_STRUCT_FUNCTION (current_function_decl)->cfg)
+ {
+ num_loops = flow_loops_find (&loops);
+ /* FIXME - Deallocating the loop causes a seg-fault. */
+#if 0
+ flow_loops_free (&loops);
+#endif
+ /* We are not concerned with the function body as a loop. */
+ if (num_loops > 1)
+ return true;
+ }
+ }
+
+ /* Borrowed this code from rest_of_handle_final() in final.c. */
+ func_name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
+ if (!patch_functions_dont_always_patch_main &&
+ func_name &&
+ strcmp("main", func_name) == 0)
+ return true;
+
+ if (patch_functions_min_instructions > 0)
+ {
+ /* Calculate the number of instructions in this function and only emit
+ function patch for instrumentation if it is greater than
+ patch_functions_min_instructions. */
+ for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+ {
+ if (INSN_P (insn))
+ ++num_insns;
+ }
+ if (num_insns < patch_functions_min_instructions)
+ return false;
+ }
+
+ return true;
+}
+
+/* Emit the 11-byte patch space for the function prologue for functions that
+ qualify. */
+static void
+ix86_output_function_prologue (FILE *file,
+ HOST_WIDE_INT size ATTRIBUTE_UNUSED)
+{
+ /* Only for 64-bit target. */
+ if (TARGET_64BIT && patch_functions_for_instrumentation)
+ {
+ patch_current_function_p = check_should_patch_current_function();
+ /* Emit the instruction 'jmp 09' followed by 9 bytes to make it 11-bytes
+ of nop. */
+ ix86_output_function_nops_prologue_epilogue (file,
+ "_function_patch_prologue",
+ ASM_BYTE"0xeb,0x09",
+ 9);
+ }
+}
+
+/* Emit the nop bytes at function prologue or return (including tail call
+ jumps). The number of nop bytes generated is at least 8.
+ Also emits a section named SECTION_NAME, which is a backpointer section
+ holding the addresses of the nop bytes in the text section.
+ SECTION_NAME is either '_function_patch_prologue' or
+ '_function_patch_epilogue'.
+ PRE_INSTRUCTIONS are the instructions, if any, at the start of the nop byte
+ sequence. NUM_REMAINING_NOPS are the number of nop bytes to fill,
+ excluding the number of bytes in PRE_INSTRUCTIONS.
+ Returns true if the function was patched, false otherwise. */
+bool
+ix86_output_function_nops_prologue_epilogue (FILE *file,
+ const char *section_name,
+ const char *pre_instructions,
+ unsigned int num_remaining_nops)
+{
+ static int labelno = 0;
+ char label[32], section_label[32];
+ section *section = NULL;
+ unsigned int num_actual_nops = num_remaining_nops - 8;
+ unsigned int section_flags = SECTION_RELRO;
+ char *section_name_comdat = NULL;
+ const char *decl_section_name = NULL;
+ size_t len;
+
+ gcc_assert (num_remaining_nops >= 8);
+
+ if (!patch_current_function_p)
+ return false;
+
+ ASM_GENERATE_INTERNAL_LABEL (label, "LFPEL", labelno);
+ ASM_GENERATE_INTERNAL_LABEL (section_label, "LFPESL", labelno++);
+
+ /* Align the start of nops to 2-byte boundary so that the 2-byte jump
+ instruction can be patched atomically at run time. */
+ ASM_OUTPUT_ALIGN (file, 1);
+
+ /* Emit nop bytes. They look like the following:
+ $LFPEL0:
+ <pre_instruction>
+ 0x90 (repeated num_actual_nops times)
+ .quad $LFPESL0
+ followed by section 'section_name' which contains the address
+ of instruction at 'label'.
+ */
+ ASM_OUTPUT_INTERNAL_LABEL (file, label);
+ if (pre_instructions)
+ fprintf (file, "%s\n", pre_instructions);
+
+ while (num_actual_nops-- > 0)
+ asm_fprintf (file, ASM_BYTE"0x90\n");
+
+ fprintf (file, ASM_QUAD);
+ assemble_name_raw (file, section_label);
+ fprintf (file, "\n");
+
+ /* Emit the backpointer section. For functions belonging to comdat group,
+ we emit a different section named '<section_name>.foo'. This section
+ is later renamed to '<section_name>' by ix86_elf_asm_named_section(). */
+ if (current_function_decl != NULL_TREE &&
+ DECL_ONE_ONLY (current_function_decl) &&
+ HAVE_COMDAT_GROUP)
+ {
+ decl_section_name =
+ TREE_STRING_POINTER (DECL_SECTION_NAME (current_function_decl));
+ len = strlen (decl_section_name) + strlen (section_name) + 1;
+ section_name_comdat = (char *) alloca (len);
+ sprintf (section_name_comdat, "%s.%s", section_name, decl_section_name);
+ section_name = section_name_comdat;
+ section_flags |= SECTION_LINKONCE;
+ }
+ section = get_section (section_name, section_flags, current_function_decl);
+ switch_to_section (section);
+ /* Align the section to 8-byte boundary. */
+ ASM_OUTPUT_ALIGN (file, 3);
+
+ /* Emit address of the start of nop bytes in the section:
+ $LFPESP0:
+ .quad $LFPEL0
+ */
+ ASM_OUTPUT_INTERNAL_LABEL (file, section_label);
+ fprintf(file, ASM_QUAD"\t");
+ assemble_name_raw (file, label);
+ fprintf (file, "\n");
+
+ /* Switching back to text section. */
+ switch_to_section (function_section (current_function_decl));
+ return true;
+}
+
+/* Strips the characters after '_function_patch_prologue' or
+ '_function_patch_epilogue' and emits the section. */
+static void
+ix86_elf_asm_named_section (const char *name, unsigned int flags,
+ tree decl)
+{
+ const char *section_name = name;
+ if (HAVE_COMDAT_GROUP && flags & SECTION_LINKONCE)
+ {
+ /* Both section names have the same length. */
+ static const int section_name_length =
+ sizeof("_function_patch_prologue") - 1;
+ if (strncmp (name, "_function_patch_prologue", section_name_length) == 0)
+ section_name = "_function_patch_prologue";
+ else if (strncmp (name, "_function_patch_epilogue",
+ section_name_length) == 0)
+ section_name = "_function_patch_epilogue";
+ }
+ default_elf_asm_named_section (section_name, flags, decl);
+}
+
/* Reset from the function's potential modifications. */
static void
@@ -22164,6 +22349,14 @@ ix86_output_call_insn (rtx insn, rtx call_op)
else
xasm = "jmp\t%A0";
+ /* Just before the sibling call, add 11-bytes of nops to patch function
+ exit: 2 bytes for 'jmp 09' and remaining 9 bytes. */
+ if (TARGET_64BIT && patch_functions_for_instrumentation)
+ ix86_output_function_nops_prologue_epilogue (asm_out_file,
+ "_function_patch_epilogue",
+ ASM_BYTE"0xeb, 0x09",
+ 9);
+
output_asm_insn (xasm, &call_op);
return "";
}
@@ -36213,9 +36406,15 @@ ix86_autovectorize_vector_sizes (void)
#undef TARGET_BUILTIN_RECIPROCAL
#define TARGET_BUILTIN_RECIPROCAL ix86_builtin_reciprocal
+#undef TARGET_ASM_FUNCTION_PROLOGUE
+#define TARGET_ASM_FUNCTION_PROLOGUE ix86_output_function_prologue
+
#undef TARGET_ASM_FUNCTION_EPILOGUE
#define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
+#undef TARGET_ASM_NAMED_SECTION
+#define TARGET_ASM_NAMED_SECTION ix86_elf_asm_named_section
+
#undef TARGET_ENCODE_SECTION_INFO
#ifndef SUBTARGET_ENCODE_SECTION_INFO
#define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 52c57fa..35b943a 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11676,7 +11676,18 @@
(define_insn "return_internal"
[(return)]
"reload_completed"
- "ret"
+{
+ if (TARGET_64BIT && patch_functions_for_instrumentation)
+ {
+ /* Emit 10 nop bytes after ret. */
+ if (ix86_output_function_nops_prologue_epilogue (asm_out_file,
+ "_function_patch_epilogue",
+ "ret",
+ 10))
+ return "";
+ }
+ return "ret";
+}
[(set_attr "length" "1")
(set_attr "atom_unit" "jeu")
(set_attr "length_immediate" "0")
@@ -11689,7 +11700,18 @@
[(return)
(unspec [(const_int 0)] UNSPEC_REP)]
"reload_completed"
- "rep\;ret"
+{
+ if (TARGET_64BIT && patch_functions_for_instrumentation)
+ {
+ /* Emit 9 nop bytes after rep;ret. */
+ if (ix86_output_function_nops_prologue_epilogue (asm_out_file,
+ "_function_patch_epilogue",
+ "rep\;ret",
+ 9))
+ return "";
+ }
+ return "rep\;ret";
+}
[(set_attr "length" "2")
(set_attr "atom_unit" "jeu")
(set_attr "length_immediate" "0")
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 8e4d51b..85228cf 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -560,3 +560,19 @@ Split 32-byte AVX unaligned load
mavx256-split-unaligned-store
Target Report Mask(AVX256_SPLIT_UNALIGNED_STORE) Save
Split 32-byte AVX unaligned store
+
+mpatch-functions-for-instrumentation
+Target RejectNegative Report Var(patch_functions_for_instrumentation) Save
+Patch function prologue and epilogue with custom NOPs for dynamic instrumentation. By default, functions with loops (controlled by -mpatch-functions-without-loop) or functions having instructions more than -mpatch-functions-min-instructions are patched.
+
+mpatch-functions-min-instructions=
+Target Report Joined UInteger Var(patch_functions_min_instructions) Init(200) Save
+Minimum number of instructions in the function without loop before the function is qualified for patching for instrumentation (for use with -mpatch-functions-for-instrumentation)
+
+mpatch-functions-ignore-loops
+Target RejectNegative Report Var(patch_functions_ignore_loops) Save
+Ignore loops when deciding whether to patch a function for instrumentation (for use with -mpatch-functions-for-instrumentation).
+
+mno-patch-functions-main-always
+Target Report RejectNegative Var(patch_functions_dont_always_patch_main) Save
+Treat 'main' as any other function and only patch it if it meets the criteria for loops and minimum number of instructions (for use with -mpatch-functions-for-instrumentation).
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-1.c b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
new file mode 100644
index 0000000..9f11945
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-1.c
@@ -0,0 +1,14 @@
+/* Verify -mpatch-functions-for-instrumentation works. */
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation" } */
+
+/* Check nop-bytes at beginning. */
+/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* Check nop-bytes at end. */
+/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+void foo() {
+ /* Dummy loop. */
+ int x = 0;
+ while (++x);
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-2.c b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
new file mode 100644
index 0000000..86c0ab7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation" } */
+
+/* Function is small to be instrumented with default values. Check there
+ aren't any nop-bytes at beginning or end of function. */
+
+/* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+void foo() {
+ int x = 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-3.c b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
new file mode 100644
index 0000000..54cc625
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation -mpatch-functions-min-instructions=0" } */
+
+/* Function should have nop-bytes with -mpatch-function-min-instructions=0.
+ Check there are nop-bytes at beginning and end of function. */
+
+/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+void foo() {
+ int x = 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-4.c b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
new file mode 100644
index 0000000..a0d8f18
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-4.c
@@ -0,0 +1,13 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation -mpatch-functions-ignore-loops" } */
+
+/* Function is too small to be patched when ignoring the loop.
+ Check there aren't any nop-bytes at beginning and end of function. */
+
+/* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+void foo() {
+ int x = 0;
+ while (++x);
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-5.c b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
new file mode 100644
index 0000000..8ac8bae
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-5.c
@@ -0,0 +1,13 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation -mpatch-functions-ignore-loops -mpatch-functions-min-instructions=0" } */
+
+/* Function should be patched with nop bytes with given options.
+ Check there are nop-bytes at beginning and end of function. */
+
+/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+void foo() {
+ int x = 0;
+ while (++x);
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-6.c b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
new file mode 100644
index 0000000..2af551a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-6.c
@@ -0,0 +1,13 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation" } */
+
+/* 'main' function should always be patched, irrespective of how small it is.
+ Check there are nop-bytes at beginning and end of main. */
+
+/* { dg-final { scan-assembler ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+int main(int argc, char **argv) {
+ int x = 0;
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/patch-functions-7.c b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
new file mode 100644
index 0000000..572a00f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/patch-functions-7.c
@@ -0,0 +1,13 @@
+/* { dg-do compile} */
+/* { dg-options "-mpatch-functions-for-instrumentation -mno-patch-functions-main-always" } */
+
+/* 'main' shouldn't be patched with the option -mno-patch-functions-main-always.
+ Check there aren't any nop-bytes at beginning and end of main. */
+
+/* { dg-final { scan-assembler-not ".byte\t0xeb,0x09(.*).byte\t0x90" } } */
+/* { dg-final { scan-assembler-not "ret(.*).byte\t0x90(.*).byte\t0x90" } } */
+
+int main(int argc, char **argv) {
+ int x = 0;
+ return 0;
+}
--
1.7.3.1
--
This patch is available for review at http://codereview.appspot.com/5416043