* [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
@ 2014-04-16 14:16 Ilya Enkovich
2014-05-06 12:14 ` Ilya Enkovich
2014-07-23 13:47 ` Jeff Law
0 siblings, 2 replies; 11+ messages in thread
From: Ilya Enkovich @ 2014-04-16 14:16 UTC (permalink / raw)
To: gcc-patches
Hi,
This patch introduces changes in call graph for Pointer Bounds Checker.
New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
- instrumentation_clone field is 1 for nodes created for instrumented version of functions
- instrumented_version points to instrumented/original node
- orig_decl holds original function declaration for instrumented nodes in case original node is removed
IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals. It is used to have proper reachability analysis.
When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function. Therefore new field appeared in cgraph_thunk_info to mark such thunks.
Does it look OK?
Bootstrapped and tested on linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-04-16 Ilya Enkovich <ilya.enkovich@intel.com>
* cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
field.
(cgraph_node): Add instrumented_version, orig_decl and
instrumentation_clone fields.
(symtab_alias_target): Allow IPA_REF_CHKP reference.
* cgraph.c (cgraph_remove_node): Fix instrumented_version
of the referenced node if any.
(dump_cgraph_node): Dump instrumentation_clone and
instrumented_version fields.
(verify_cgraph_node): Check correctness of IPA_REF_CHKP
references and instrumentation thunks.
* cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
reference.
(cgraph_rebuild_references): Likewise.
* cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
calling instrumneted function version.
* ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
(ipa_ref): increase size of use field.
* ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
* lto-cgraph.c (lto_output_node): Output instrumentation_clone,
thunk.add_pointer_bounds_args and orig_decl field.
(lto_output_ref): Adjust to new ipa_ref::use field size.
(input_overwrite_node): Read instrumentation_clone field.
(input_node): Read thunk.add_pointer_bounds_args and orig_decl
fields.
(input_ref): Adjust to new ipa_ref::use field size.
(input_cgraph_1): Compute instrumented_version fields and restore
IDENTIFIER_TRANSPARENT_ALIAS chains.
* lto-streamer.h (LTO_minor_version): Change minor version from
0 to 1.
* ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
clone as address taken if the original one is address taken.
(cgraph_externally_visible_p): Mark instrumented 'main' as
externally visible.
(function_and_variable_visibility): Filter instrumentation
thunks.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index be3661a..6210c68 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1828,6 +1828,12 @@ cgraph_remove_node (struct cgraph_node *node)
}
cgraph_n_nodes--;
+ if (node->instrumented_version)
+ {
+ node->instrumented_version->instrumented_version = NULL;
+ node->instrumented_version = NULL;
+ }
+
/* Clear out the node to NULL all pointers and add the node to the free
list. */
memset (node, 0, sizeof (*node));
@@ -2070,6 +2076,11 @@ dump_cgraph_node (FILE *f, struct cgraph_node *node)
if (indirect_calls_count)
fprintf (f, " Has %i outgoing edges for indirect calls.\n",
indirect_calls_count);
+
+ if (node->instrumentation_clone)
+ fprintf (f, " Is instrumented version.\n");
+ else if (node->instrumented_version)
+ fprintf (f, " Has instrumented version.\n");
}
@@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
}
for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
i, ref); i++)
- if (ref->use != IPA_REF_ALIAS)
+ if (ref->use == IPA_REF_CHKP)
+ ;
+ else if (ref->use != IPA_REF_ALIAS)
{
error ("Alias has non-alias reference");
error_found = true;
@@ -2868,6 +2881,35 @@ verify_cgraph_node (struct cgraph_node *node)
error_found = true;
}
}
+
+ /* Check all nodes reference their instrumented versions. */
+ if (node->analyzed
+ && node->instrumented_version
+ && !node->instrumentation_clone)
+ {
+ bool ref_found = false;
+ int i;
+ struct ipa_ref *ref;
+
+ for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
+ i, ref); i++)
+ if (ref->use == IPA_REF_CHKP)
+ {
+ if (ref_found)
+ {
+ error ("Node has more than one chkp reference");
+ error_found = true;
+ }
+ ref_found = true;
+ }
+
+ if (!ref_found)
+ {
+ error ("Analyzed node has no reference to instrumented version");
+ error_found = true;
+ }
+ }
+
if (node->analyzed && node->thunk.thunk_p)
{
if (!node->callees)
@@ -2885,6 +2927,12 @@ verify_cgraph_node (struct cgraph_node *node)
error ("Thunk is not supposed to have body");
error_found = true;
}
+ if (node->thunk.add_pointer_bounds_args
+ && node->callees->callee != node->instrumented_version)
+ {
+ error ("Instrumentation thunk has wrong edge callee");
+ error_found = true;
+ }
}
else if (node->analyzed && gimple_has_body_p (node->decl)
&& !TREE_ASM_WRITTEN (node->decl)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a6a51cf..5e702a7 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
tree alias;
bool this_adjusting;
bool virtual_offset_p;
+ bool add_pointer_bounds_args;
/* Set to true when alias node is thunk. */
bool thunk_p;
};
@@ -373,6 +374,13 @@ public:
struct cgraph_node *prev_sibling_clone;
struct cgraph_node *clones;
struct cgraph_node *clone_of;
+ /* If instrumentation_clone is 1 then instrumented_version points
+ to the original function used to make instrumented version.
+ Otherwise points to instrumented version of the function. */
+ struct cgraph_node *instrumented_version;
+ /* If instrumentation_clone is 1 then orig_decl is the original
+ function declaration. */
+ tree orig_decl;
/* For functions with many calls sites it holds map from call expression
to the edge to speed up cgraph_edge function. */
htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
@@ -433,6 +441,9 @@ public:
/* True if this decl calls a COMDAT-local function. This is set up in
compute_inline_parameters and inline_call. */
unsigned calls_comdat_local : 1;
+ /* True when function is clone created for Pointer Bounds Checker
+ instrumentation. */
+ unsigned instrumentation_clone : 1;
};
@@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
{
struct ipa_ref *ref;
ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
+ if (ref->use == IPA_REF_CHKP)
+ ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
gcc_checking_assert (ref->use == IPA_REF_ALIAS);
return ref->referred;
}
diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index 19961e2..a2b2106 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
record_eh_tables (node, cfun);
gcc_assert (!node->global.inlined_to);
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
+
return 0;
}
@@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
ipa_record_stmt_references (node, gsi_stmt (gsi));
}
record_eh_tables (node, cfun);
+
+
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
}
namespace {
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 06283fc..ceb4060 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
struct ipa_ref *ref;
for (e = node->callers; e;)
- if (e->caller->thunk.thunk_p)
+ if (e->caller->thunk.thunk_p
+ && !e->caller->thunk.add_pointer_bounds_args)
{
struct cgraph_node *thunk = e->caller;
diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
index 6aa41e6..3a055d9 100644
--- a/gcc/ipa-ref.c
+++ b/gcc/ipa-ref.c
@@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
#include "cgraph.h"
#include "ipa-utils.h"
-static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
+static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
/* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index 4ce5f8d..d0df0bf 100644
--- a/gcc/ipa-ref.h
+++ b/gcc/ipa-ref.h
@@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
IPA_REF_LOAD,
IPA_REF_STORE,
IPA_REF_ADDR,
- IPA_REF_ALIAS
+ IPA_REF_ALIAS,
+ IPA_REF_CHKP
};
/* Record of reference in callgraph or varpool. */
@@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
gimple stmt;
unsigned int lto_stmt_uid;
unsigned int referred_index;
- ENUM_BITFIELD (ipa_ref_use) use:2;
+ ENUM_BITFIELD (ipa_ref_use) use:3;
unsigned int speculative:1;
};
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 5ab3aed..1d7fa35 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
cgraph_node_remove_callees (node);
ipa_remove_all_references (&node->ref_list);
changed = true;
+ if (node->thunk.thunk_p
+ && node->thunk.add_pointer_bounds_args)
+ {
+ node->thunk.thunk_p = false;
+ node->thunk.add_pointer_bounds_args = false;
+ }
}
}
else
@@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
if (node->address_taken
&& !node->used_from_other_partition)
{
- if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
+ if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
+ && (!node->instrumentation_clone
+ || !node->instrumented_version
+ || !node->instrumented_version->address_taken))
{
if (file)
fprintf (file, " %s", node->name ());
@@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
if (MAIN_NAME_P (DECL_NAME (node->decl)))
return true;
+ if (node->instrumentation_clone
+ && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
+ return true;
+
return false;
}
@@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
}
if (node->thunk.thunk_p
+ && !node->thunk.add_pointer_bounds_args
&& TREE_PUBLIC (node->decl))
{
struct cgraph_node *decl_node = node;
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 999ce3d..58105f0 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
bp_pack_enum (&bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN, node->resolution);
+ bp_pack_value (&bp, node->instrumentation_clone, 1);
streamer_write_bitpack (&bp);
if (node->thunk.thunk_p && !boundary_p)
@@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
streamer_write_uhwi_stream
(ob->main_stream,
1 + (node->thunk.this_adjusting != 0) * 2
- + (node->thunk.virtual_offset_p != 0) * 4);
+ + (node->thunk.virtual_offset_p != 0) * 4
+ + (node->thunk.add_pointer_bounds_args != 0) * 8);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
}
streamer_write_hwi_stream (ob->main_stream, node->profile_id);
+
+ if (node->instrumentation_clone)
+ lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
}
/* Output the varpool NODE to OB.
@@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
struct cgraph_node *node;
bp = bitpack_create (ob->main_stream);
- bp_pack_value (&bp, ref->use, 2);
+ bp_pack_value (&bp, ref->use, 3);
bp_pack_value (&bp, ref->speculative, 1);
streamer_write_bitpack (&bp);
nref = lto_symtab_encoder_lookup (encoder, ref->referred);
@@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
node->thunk.thunk_p = bp_unpack_value (bp, 1);
node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN);
+ node->instrumentation_clone = bp_unpack_value (bp, 1);
gcc_assert (flag_ltrans
|| (!node->in_other_partition
&& !node->used_from_other_partition));
@@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
node->thunk.this_adjusting = (type & 2);
node->thunk.virtual_value = virtual_value;
node->thunk.virtual_offset_p = (type & 4);
+ node->thunk.add_pointer_bounds_args = (type & 8);
}
if (node->alias && !node->analyzed && node->weakref)
node->alias_target = get_alias_symbol (node->decl);
node->profile_id = streamer_read_hwi (ib);
+
+ if (node->instrumentation_clone)
+ {
+ decl_index = streamer_read_uhwi (ib);
+ fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
+ node->orig_decl = fn_decl;
+ }
+
return node;
}
@@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
struct ipa_ref *ref;
bp = streamer_read_bitpack (ib);
- use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
+ use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
node = nodes[streamer_read_hwi (ib)];
ref = ipa_record_reference (referring_node, node, use, NULL);
@@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
else
cnode->global.inlined_to = NULL;
+
+ /* Compute instrumented_version. */
+ if (cnode->instrumentation_clone)
+ {
+ gcc_assert (cnode->orig_decl);
+
+ cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
+ if (cnode->instrumented_version)
+ cnode->instrumented_version->instrumented_version = cnode;
+
+ /* Restore decl names reference. */
+ if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
+ && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
+ TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
+ = DECL_ASSEMBLER_NAME (cnode->orig_decl);
+ }
}
ref = (int) (intptr_t) node->same_comdat_group;
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 51b1903..62a5fe0 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
#define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
#define LTO_major_version 3
-#define LTO_minor_version 0
+#define LTO_minor_version 1
typedef unsigned char lto_decl_flags_t;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-04-16 14:16 [PATCH, Pointer Bounds Checker 9/x] Cgraph extension Ilya Enkovich
@ 2014-05-06 12:14 ` Ilya Enkovich
2014-06-27 8:12 ` Ilya Enkovich
2014-07-23 13:47 ` Jeff Law
1 sibling, 1 reply; 11+ messages in thread
From: Ilya Enkovich @ 2014-05-06 12:14 UTC (permalink / raw)
To: gcc-patches
Ping
2014-04-16 18:03 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> Hi,
>
> This patch introduces changes in call graph for Pointer Bounds Checker.
>
> New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
> - instrumentation_clone field is 1 for nodes created for instrumented version of functions
> - instrumented_version points to instrumented/original node
> - orig_decl holds original function declaration for instrumented nodes in case original node is removed
>
> IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals. It is used to have proper reachability analysis.
>
> When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function. Therefore new field appeared in cgraph_thunk_info to mark such thunks.
>
> Does it look OK?
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-04-16 Ilya Enkovich <ilya.enkovich@intel.com>
>
> * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
> field.
> (cgraph_node): Add instrumented_version, orig_decl and
> instrumentation_clone fields.
> (symtab_alias_target): Allow IPA_REF_CHKP reference.
> * cgraph.c (cgraph_remove_node): Fix instrumented_version
> of the referenced node if any.
> (dump_cgraph_node): Dump instrumentation_clone and
> instrumented_version fields.
> (verify_cgraph_node): Check correctness of IPA_REF_CHKP
> references and instrumentation thunks.
> * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
> reference.
> (cgraph_rebuild_references): Likewise.
> * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
> calling instrumneted function version.
> * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
> (ipa_ref): increase size of use field.
> * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
> * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
> thunk.add_pointer_bounds_args and orig_decl field.
> (lto_output_ref): Adjust to new ipa_ref::use field size.
> (input_overwrite_node): Read instrumentation_clone field.
> (input_node): Read thunk.add_pointer_bounds_args and orig_decl
> fields.
> (input_ref): Adjust to new ipa_ref::use field size.
> (input_cgraph_1): Compute instrumented_version fields and restore
> IDENTIFIER_TRANSPARENT_ALIAS chains.
> * lto-streamer.h (LTO_minor_version): Change minor version from
> 0 to 1.
> * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
> clone as address taken if the original one is address taken.
> (cgraph_externally_visible_p): Mark instrumented 'main' as
> externally visible.
> (function_and_variable_visibility): Filter instrumentation
> thunks.
>
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index be3661a..6210c68 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -1828,6 +1828,12 @@ cgraph_remove_node (struct cgraph_node *node)
> }
> cgraph_n_nodes--;
>
> + if (node->instrumented_version)
> + {
> + node->instrumented_version->instrumented_version = NULL;
> + node->instrumented_version = NULL;
> + }
> +
> /* Clear out the node to NULL all pointers and add the node to the free
> list. */
> memset (node, 0, sizeof (*node));
> @@ -2070,6 +2076,11 @@ dump_cgraph_node (FILE *f, struct cgraph_node *node)
> if (indirect_calls_count)
> fprintf (f, " Has %i outgoing edges for indirect calls.\n",
> indirect_calls_count);
> +
> + if (node->instrumentation_clone)
> + fprintf (f, " Is instrumented version.\n");
> + else if (node->instrumented_version)
> + fprintf (f, " Has instrumented version.\n");
> }
>
>
> @@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
> }
> for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
> i, ref); i++)
> - if (ref->use != IPA_REF_ALIAS)
> + if (ref->use == IPA_REF_CHKP)
> + ;
> + else if (ref->use != IPA_REF_ALIAS)
> {
> error ("Alias has non-alias reference");
> error_found = true;
> @@ -2868,6 +2881,35 @@ verify_cgraph_node (struct cgraph_node *node)
> error_found = true;
> }
> }
> +
> + /* Check all nodes reference their instrumented versions. */
> + if (node->analyzed
> + && node->instrumented_version
> + && !node->instrumentation_clone)
> + {
> + bool ref_found = false;
> + int i;
> + struct ipa_ref *ref;
> +
> + for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
> + i, ref); i++)
> + if (ref->use == IPA_REF_CHKP)
> + {
> + if (ref_found)
> + {
> + error ("Node has more than one chkp reference");
> + error_found = true;
> + }
> + ref_found = true;
> + }
> +
> + if (!ref_found)
> + {
> + error ("Analyzed node has no reference to instrumented version");
> + error_found = true;
> + }
> + }
> +
> if (node->analyzed && node->thunk.thunk_p)
> {
> if (!node->callees)
> @@ -2885,6 +2927,12 @@ verify_cgraph_node (struct cgraph_node *node)
> error ("Thunk is not supposed to have body");
> error_found = true;
> }
> + if (node->thunk.add_pointer_bounds_args
> + && node->callees->callee != node->instrumented_version)
> + {
> + error ("Instrumentation thunk has wrong edge callee");
> + error_found = true;
> + }
> }
> else if (node->analyzed && gimple_has_body_p (node->decl)
> && !TREE_ASM_WRITTEN (node->decl)
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index a6a51cf..5e702a7 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
> tree alias;
> bool this_adjusting;
> bool virtual_offset_p;
> + bool add_pointer_bounds_args;
> /* Set to true when alias node is thunk. */
> bool thunk_p;
> };
> @@ -373,6 +374,13 @@ public:
> struct cgraph_node *prev_sibling_clone;
> struct cgraph_node *clones;
> struct cgraph_node *clone_of;
> + /* If instrumentation_clone is 1 then instrumented_version points
> + to the original function used to make instrumented version.
> + Otherwise points to instrumented version of the function. */
> + struct cgraph_node *instrumented_version;
> + /* If instrumentation_clone is 1 then orig_decl is the original
> + function declaration. */
> + tree orig_decl;
> /* For functions with many calls sites it holds map from call expression
> to the edge to speed up cgraph_edge function. */
> htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
> @@ -433,6 +441,9 @@ public:
> /* True if this decl calls a COMDAT-local function. This is set up in
> compute_inline_parameters and inline_call. */
> unsigned calls_comdat_local : 1;
> + /* True when function is clone created for Pointer Bounds Checker
> + instrumentation. */
> + unsigned instrumentation_clone : 1;
> };
>
>
> @@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
> {
> struct ipa_ref *ref;
> ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
> + if (ref->use == IPA_REF_CHKP)
> + ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
> gcc_checking_assert (ref->use == IPA_REF_ALIAS);
> return ref->referred;
> }
> diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
> index 19961e2..a2b2106 100644
> --- a/gcc/cgraphbuild.c
> +++ b/gcc/cgraphbuild.c
> @@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
> record_eh_tables (node, cfun);
> gcc_assert (!node->global.inlined_to);
>
> + if (node->instrumented_version
> + && !node->instrumentation_clone)
> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> +
> return 0;
> }
>
> @@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
> ipa_record_stmt_references (node, gsi_stmt (gsi));
> }
> record_eh_tables (node, cfun);
> +
> +
> + if (node->instrumented_version
> + && !node->instrumentation_clone)
> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> }
>
> namespace {
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 06283fc..ceb4060 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
> struct ipa_ref *ref;
>
> for (e = node->callers; e;)
> - if (e->caller->thunk.thunk_p)
> + if (e->caller->thunk.thunk_p
> + && !e->caller->thunk.add_pointer_bounds_args)
> {
> struct cgraph_node *thunk = e->caller;
>
> diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
> index 6aa41e6..3a055d9 100644
> --- a/gcc/ipa-ref.c
> +++ b/gcc/ipa-ref.c
> @@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
> #include "cgraph.h"
> #include "ipa-utils.h"
>
> -static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
> +static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
>
> /* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
> to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
> diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
> index 4ce5f8d..d0df0bf 100644
> --- a/gcc/ipa-ref.h
> +++ b/gcc/ipa-ref.h
> @@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
> IPA_REF_LOAD,
> IPA_REF_STORE,
> IPA_REF_ADDR,
> - IPA_REF_ALIAS
> + IPA_REF_ALIAS,
> + IPA_REF_CHKP
> };
>
> /* Record of reference in callgraph or varpool. */
> @@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
> gimple stmt;
> unsigned int lto_stmt_uid;
> unsigned int referred_index;
> - ENUM_BITFIELD (ipa_ref_use) use:2;
> + ENUM_BITFIELD (ipa_ref_use) use:3;
> unsigned int speculative:1;
> };
>
> diff --git a/gcc/ipa.c b/gcc/ipa.c
> index 5ab3aed..1d7fa35 100644
> --- a/gcc/ipa.c
> +++ b/gcc/ipa.c
> @@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
> cgraph_node_remove_callees (node);
> ipa_remove_all_references (&node->ref_list);
> changed = true;
> + if (node->thunk.thunk_p
> + && node->thunk.add_pointer_bounds_args)
> + {
> + node->thunk.thunk_p = false;
> + node->thunk.add_pointer_bounds_args = false;
> + }
> }
> }
> else
> @@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
> if (node->address_taken
> && !node->used_from_other_partition)
> {
> - if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
> + if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
> + && (!node->instrumentation_clone
> + || !node->instrumented_version
> + || !node->instrumented_version->address_taken))
> {
> if (file)
> fprintf (file, " %s", node->name ());
> @@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
> if (MAIN_NAME_P (DECL_NAME (node->decl)))
> return true;
>
> + if (node->instrumentation_clone
> + && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
> + return true;
> +
> return false;
> }
>
> @@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
> }
>
> if (node->thunk.thunk_p
> + && !node->thunk.add_pointer_bounds_args
> && TREE_PUBLIC (node->decl))
> {
> struct cgraph_node *decl_node = node;
> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> index 999ce3d..58105f0 100644
> --- a/gcc/lto-cgraph.c
> +++ b/gcc/lto-cgraph.c
> @@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
> bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
> bp_pack_enum (&bp, ld_plugin_symbol_resolution,
> LDPR_NUM_KNOWN, node->resolution);
> + bp_pack_value (&bp, node->instrumentation_clone, 1);
> streamer_write_bitpack (&bp);
>
> if (node->thunk.thunk_p && !boundary_p)
> @@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
> streamer_write_uhwi_stream
> (ob->main_stream,
> 1 + (node->thunk.this_adjusting != 0) * 2
> - + (node->thunk.virtual_offset_p != 0) * 4);
> + + (node->thunk.virtual_offset_p != 0) * 4
> + + (node->thunk.add_pointer_bounds_args != 0) * 8);
> streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
> streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
> }
> streamer_write_hwi_stream (ob->main_stream, node->profile_id);
> +
> + if (node->instrumentation_clone)
> + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
> }
>
> /* Output the varpool NODE to OB.
> @@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
> struct cgraph_node *node;
>
> bp = bitpack_create (ob->main_stream);
> - bp_pack_value (&bp, ref->use, 2);
> + bp_pack_value (&bp, ref->use, 3);
> bp_pack_value (&bp, ref->speculative, 1);
> streamer_write_bitpack (&bp);
> nref = lto_symtab_encoder_lookup (encoder, ref->referred);
> @@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
> node->thunk.thunk_p = bp_unpack_value (bp, 1);
> node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
> LDPR_NUM_KNOWN);
> + node->instrumentation_clone = bp_unpack_value (bp, 1);
> gcc_assert (flag_ltrans
> || (!node->in_other_partition
> && !node->used_from_other_partition));
> @@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
> node->thunk.this_adjusting = (type & 2);
> node->thunk.virtual_value = virtual_value;
> node->thunk.virtual_offset_p = (type & 4);
> + node->thunk.add_pointer_bounds_args = (type & 8);
> }
> if (node->alias && !node->analyzed && node->weakref)
> node->alias_target = get_alias_symbol (node->decl);
> node->profile_id = streamer_read_hwi (ib);
> +
> + if (node->instrumentation_clone)
> + {
> + decl_index = streamer_read_uhwi (ib);
> + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
> + node->orig_decl = fn_decl;
> + }
> +
> return node;
> }
>
> @@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
> struct ipa_ref *ref;
>
> bp = streamer_read_bitpack (ib);
> - use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
> + use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
> speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
> node = nodes[streamer_read_hwi (ib)];
> ref = ipa_record_reference (referring_node, node, use, NULL);
> @@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
> cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
> else
> cnode->global.inlined_to = NULL;
> +
> + /* Compute instrumented_version. */
> + if (cnode->instrumentation_clone)
> + {
> + gcc_assert (cnode->orig_decl);
> +
> + cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
> + if (cnode->instrumented_version)
> + cnode->instrumented_version->instrumented_version = cnode;
> +
> + /* Restore decl names reference. */
> + if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
> + && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
> + TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
> + = DECL_ASSEMBLER_NAME (cnode->orig_decl);
> + }
> }
>
> ref = (int) (intptr_t) node->same_comdat_group;
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 51b1903..62a5fe0 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
> #define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
>
> #define LTO_major_version 3
> -#define LTO_minor_version 0
> +#define LTO_minor_version 1
>
> typedef unsigned char lto_decl_flags_t;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-05-06 12:14 ` Ilya Enkovich
@ 2014-06-27 8:12 ` Ilya Enkovich
0 siblings, 0 replies; 11+ messages in thread
From: Ilya Enkovich @ 2014-06-27 8:12 UTC (permalink / raw)
To: gcc-patches
Ping
2014-05-06 16:13 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> Ping
>
> 2014-04-16 18:03 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> Hi,
>>
>> This patch introduces changes in call graph for Pointer Bounds Checker.
>>
>> New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
>> - instrumentation_clone field is 1 for nodes created for instrumented version of functions
>> - instrumented_version points to instrumented/original node
>> - orig_decl holds original function declaration for instrumented nodes in case original node is removed
>>
>> IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals. It is used to have proper reachability analysis.
>>
>> When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function. Therefore new field appeared in cgraph_thunk_info to mark such thunks.
>>
>> Does it look OK?
>>
>> Bootstrapped and tested on linux-x86_64.
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-04-16 Ilya Enkovich <ilya.enkovich@intel.com>
>>
>> * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
>> field.
>> (cgraph_node): Add instrumented_version, orig_decl and
>> instrumentation_clone fields.
>> (symtab_alias_target): Allow IPA_REF_CHKP reference.
>> * cgraph.c (cgraph_remove_node): Fix instrumented_version
>> of the referenced node if any.
>> (dump_cgraph_node): Dump instrumentation_clone and
>> instrumented_version fields.
>> (verify_cgraph_node): Check correctness of IPA_REF_CHKP
>> references and instrumentation thunks.
>> * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
>> reference.
>> (cgraph_rebuild_references): Likewise.
>> * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
>> calling instrumneted function version.
>> * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
>> (ipa_ref): increase size of use field.
>> * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
>> * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
>> thunk.add_pointer_bounds_args and orig_decl field.
>> (lto_output_ref): Adjust to new ipa_ref::use field size.
>> (input_overwrite_node): Read instrumentation_clone field.
>> (input_node): Read thunk.add_pointer_bounds_args and orig_decl
>> fields.
>> (input_ref): Adjust to new ipa_ref::use field size.
>> (input_cgraph_1): Compute instrumented_version fields and restore
>> IDENTIFIER_TRANSPARENT_ALIAS chains.
>> * lto-streamer.h (LTO_minor_version): Change minor version from
>> 0 to 1.
>> * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
>> clone as address taken if the original one is address taken.
>> (cgraph_externally_visible_p): Mark instrumented 'main' as
>> externally visible.
>> (function_and_variable_visibility): Filter instrumentation
>> thunks.
>>
>>
>> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
>> index be3661a..6210c68 100644
>> --- a/gcc/cgraph.c
>> +++ b/gcc/cgraph.c
>> @@ -1828,6 +1828,12 @@ cgraph_remove_node (struct cgraph_node *node)
>> }
>> cgraph_n_nodes--;
>>
>> + if (node->instrumented_version)
>> + {
>> + node->instrumented_version->instrumented_version = NULL;
>> + node->instrumented_version = NULL;
>> + }
>> +
>> /* Clear out the node to NULL all pointers and add the node to the free
>> list. */
>> memset (node, 0, sizeof (*node));
>> @@ -2070,6 +2076,11 @@ dump_cgraph_node (FILE *f, struct cgraph_node *node)
>> if (indirect_calls_count)
>> fprintf (f, " Has %i outgoing edges for indirect calls.\n",
>> indirect_calls_count);
>> +
>> + if (node->instrumentation_clone)
>> + fprintf (f, " Is instrumented version.\n");
>> + else if (node->instrumented_version)
>> + fprintf (f, " Has instrumented version.\n");
>> }
>>
>>
>> @@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
>> }
>> for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
>> i, ref); i++)
>> - if (ref->use != IPA_REF_ALIAS)
>> + if (ref->use == IPA_REF_CHKP)
>> + ;
>> + else if (ref->use != IPA_REF_ALIAS)
>> {
>> error ("Alias has non-alias reference");
>> error_found = true;
>> @@ -2868,6 +2881,35 @@ verify_cgraph_node (struct cgraph_node *node)
>> error_found = true;
>> }
>> }
>> +
>> + /* Check all nodes reference their instrumented versions. */
>> + if (node->analyzed
>> + && node->instrumented_version
>> + && !node->instrumentation_clone)
>> + {
>> + bool ref_found = false;
>> + int i;
>> + struct ipa_ref *ref;
>> +
>> + for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
>> + i, ref); i++)
>> + if (ref->use == IPA_REF_CHKP)
>> + {
>> + if (ref_found)
>> + {
>> + error ("Node has more than one chkp reference");
>> + error_found = true;
>> + }
>> + ref_found = true;
>> + }
>> +
>> + if (!ref_found)
>> + {
>> + error ("Analyzed node has no reference to instrumented version");
>> + error_found = true;
>> + }
>> + }
>> +
>> if (node->analyzed && node->thunk.thunk_p)
>> {
>> if (!node->callees)
>> @@ -2885,6 +2927,12 @@ verify_cgraph_node (struct cgraph_node *node)
>> error ("Thunk is not supposed to have body");
>> error_found = true;
>> }
>> + if (node->thunk.add_pointer_bounds_args
>> + && node->callees->callee != node->instrumented_version)
>> + {
>> + error ("Instrumentation thunk has wrong edge callee");
>> + error_found = true;
>> + }
>> }
>> else if (node->analyzed && gimple_has_body_p (node->decl)
>> && !TREE_ASM_WRITTEN (node->decl)
>> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
>> index a6a51cf..5e702a7 100644
>> --- a/gcc/cgraph.h
>> +++ b/gcc/cgraph.h
>> @@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
>> tree alias;
>> bool this_adjusting;
>> bool virtual_offset_p;
>> + bool add_pointer_bounds_args;
>> /* Set to true when alias node is thunk. */
>> bool thunk_p;
>> };
>> @@ -373,6 +374,13 @@ public:
>> struct cgraph_node *prev_sibling_clone;
>> struct cgraph_node *clones;
>> struct cgraph_node *clone_of;
>> + /* If instrumentation_clone is 1 then instrumented_version points
>> + to the original function used to make instrumented version.
>> + Otherwise points to instrumented version of the function. */
>> + struct cgraph_node *instrumented_version;
>> + /* If instrumentation_clone is 1 then orig_decl is the original
>> + function declaration. */
>> + tree orig_decl;
>> /* For functions with many calls sites it holds map from call expression
>> to the edge to speed up cgraph_edge function. */
>> htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
>> @@ -433,6 +441,9 @@ public:
>> /* True if this decl calls a COMDAT-local function. This is set up in
>> compute_inline_parameters and inline_call. */
>> unsigned calls_comdat_local : 1;
>> + /* True when function is clone created for Pointer Bounds Checker
>> + instrumentation. */
>> + unsigned instrumentation_clone : 1;
>> };
>>
>>
>> @@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
>> {
>> struct ipa_ref *ref;
>> ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
>> + if (ref->use == IPA_REF_CHKP)
>> + ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
>> gcc_checking_assert (ref->use == IPA_REF_ALIAS);
>> return ref->referred;
>> }
>> diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
>> index 19961e2..a2b2106 100644
>> --- a/gcc/cgraphbuild.c
>> +++ b/gcc/cgraphbuild.c
>> @@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
>> record_eh_tables (node, cfun);
>> gcc_assert (!node->global.inlined_to);
>>
>> + if (node->instrumented_version
>> + && !node->instrumentation_clone)
>> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
>> +
>> return 0;
>> }
>>
>> @@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
>> ipa_record_stmt_references (node, gsi_stmt (gsi));
>> }
>> record_eh_tables (node, cfun);
>> +
>> +
>> + if (node->instrumented_version
>> + && !node->instrumentation_clone)
>> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
>> }
>>
>> namespace {
>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index 06283fc..ceb4060 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
>> struct ipa_ref *ref;
>>
>> for (e = node->callers; e;)
>> - if (e->caller->thunk.thunk_p)
>> + if (e->caller->thunk.thunk_p
>> + && !e->caller->thunk.add_pointer_bounds_args)
>> {
>> struct cgraph_node *thunk = e->caller;
>>
>> diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
>> index 6aa41e6..3a055d9 100644
>> --- a/gcc/ipa-ref.c
>> +++ b/gcc/ipa-ref.c
>> @@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "cgraph.h"
>> #include "ipa-utils.h"
>>
>> -static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
>> +static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
>>
>> /* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
>> to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
>> diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
>> index 4ce5f8d..d0df0bf 100644
>> --- a/gcc/ipa-ref.h
>> +++ b/gcc/ipa-ref.h
>> @@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
>> IPA_REF_LOAD,
>> IPA_REF_STORE,
>> IPA_REF_ADDR,
>> - IPA_REF_ALIAS
>> + IPA_REF_ALIAS,
>> + IPA_REF_CHKP
>> };
>>
>> /* Record of reference in callgraph or varpool. */
>> @@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
>> gimple stmt;
>> unsigned int lto_stmt_uid;
>> unsigned int referred_index;
>> - ENUM_BITFIELD (ipa_ref_use) use:2;
>> + ENUM_BITFIELD (ipa_ref_use) use:3;
>> unsigned int speculative:1;
>> };
>>
>> diff --git a/gcc/ipa.c b/gcc/ipa.c
>> index 5ab3aed..1d7fa35 100644
>> --- a/gcc/ipa.c
>> +++ b/gcc/ipa.c
>> @@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
>> cgraph_node_remove_callees (node);
>> ipa_remove_all_references (&node->ref_list);
>> changed = true;
>> + if (node->thunk.thunk_p
>> + && node->thunk.add_pointer_bounds_args)
>> + {
>> + node->thunk.thunk_p = false;
>> + node->thunk.add_pointer_bounds_args = false;
>> + }
>> }
>> }
>> else
>> @@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
>> if (node->address_taken
>> && !node->used_from_other_partition)
>> {
>> - if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
>> + if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
>> + && (!node->instrumentation_clone
>> + || !node->instrumented_version
>> + || !node->instrumented_version->address_taken))
>> {
>> if (file)
>> fprintf (file, " %s", node->name ());
>> @@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
>> if (MAIN_NAME_P (DECL_NAME (node->decl)))
>> return true;
>>
>> + if (node->instrumentation_clone
>> + && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
>> + return true;
>> +
>> return false;
>> }
>>
>> @@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
>> }
>>
>> if (node->thunk.thunk_p
>> + && !node->thunk.add_pointer_bounds_args
>> && TREE_PUBLIC (node->decl))
>> {
>> struct cgraph_node *decl_node = node;
>> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
>> index 999ce3d..58105f0 100644
>> --- a/gcc/lto-cgraph.c
>> +++ b/gcc/lto-cgraph.c
>> @@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>> bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
>> bp_pack_enum (&bp, ld_plugin_symbol_resolution,
>> LDPR_NUM_KNOWN, node->resolution);
>> + bp_pack_value (&bp, node->instrumentation_clone, 1);
>> streamer_write_bitpack (&bp);
>>
>> if (node->thunk.thunk_p && !boundary_p)
>> @@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>> streamer_write_uhwi_stream
>> (ob->main_stream,
>> 1 + (node->thunk.this_adjusting != 0) * 2
>> - + (node->thunk.virtual_offset_p != 0) * 4);
>> + + (node->thunk.virtual_offset_p != 0) * 4
>> + + (node->thunk.add_pointer_bounds_args != 0) * 8);
>> streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
>> streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
>> }
>> streamer_write_hwi_stream (ob->main_stream, node->profile_id);
>> +
>> + if (node->instrumentation_clone)
>> + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
>> }
>>
>> /* Output the varpool NODE to OB.
>> @@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
>> struct cgraph_node *node;
>>
>> bp = bitpack_create (ob->main_stream);
>> - bp_pack_value (&bp, ref->use, 2);
>> + bp_pack_value (&bp, ref->use, 3);
>> bp_pack_value (&bp, ref->speculative, 1);
>> streamer_write_bitpack (&bp);
>> nref = lto_symtab_encoder_lookup (encoder, ref->referred);
>> @@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
>> node->thunk.thunk_p = bp_unpack_value (bp, 1);
>> node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
>> LDPR_NUM_KNOWN);
>> + node->instrumentation_clone = bp_unpack_value (bp, 1);
>> gcc_assert (flag_ltrans
>> || (!node->in_other_partition
>> && !node->used_from_other_partition));
>> @@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
>> node->thunk.this_adjusting = (type & 2);
>> node->thunk.virtual_value = virtual_value;
>> node->thunk.virtual_offset_p = (type & 4);
>> + node->thunk.add_pointer_bounds_args = (type & 8);
>> }
>> if (node->alias && !node->analyzed && node->weakref)
>> node->alias_target = get_alias_symbol (node->decl);
>> node->profile_id = streamer_read_hwi (ib);
>> +
>> + if (node->instrumentation_clone)
>> + {
>> + decl_index = streamer_read_uhwi (ib);
>> + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
>> + node->orig_decl = fn_decl;
>> + }
>> +
>> return node;
>> }
>>
>> @@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
>> struct ipa_ref *ref;
>>
>> bp = streamer_read_bitpack (ib);
>> - use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
>> + use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
>> speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
>> node = nodes[streamer_read_hwi (ib)];
>> ref = ipa_record_reference (referring_node, node, use, NULL);
>> @@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
>> cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
>> else
>> cnode->global.inlined_to = NULL;
>> +
>> + /* Compute instrumented_version. */
>> + if (cnode->instrumentation_clone)
>> + {
>> + gcc_assert (cnode->orig_decl);
>> +
>> + cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
>> + if (cnode->instrumented_version)
>> + cnode->instrumented_version->instrumented_version = cnode;
>> +
>> + /* Restore decl names reference. */
>> + if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
>> + && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
>> + TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
>> + = DECL_ASSEMBLER_NAME (cnode->orig_decl);
>> + }
>> }
>>
>> ref = (int) (intptr_t) node->same_comdat_group;
>> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
>> index 51b1903..62a5fe0 100644
>> --- a/gcc/lto-streamer.h
>> +++ b/gcc/lto-streamer.h
>> @@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
>> #define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
>>
>> #define LTO_major_version 3
>> -#define LTO_minor_version 0
>> +#define LTO_minor_version 1
>>
>> typedef unsigned char lto_decl_flags_t;
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-04-16 14:16 [PATCH, Pointer Bounds Checker 9/x] Cgraph extension Ilya Enkovich
2014-05-06 12:14 ` Ilya Enkovich
@ 2014-07-23 13:47 ` Jeff Law
2014-07-24 9:59 ` Ilya Enkovich
1 sibling, 1 reply; 11+ messages in thread
From: Jeff Law @ 2014-07-23 13:47 UTC (permalink / raw)
To: Ilya Enkovich, gcc-patches
On 04/16/14 08:03, Ilya Enkovich wrote:
> Hi,
>
> This patch introduces changes in call graph for Pointer Bounds Checker.
>
> New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
> - instrumentation_clone field is 1 for nodes created for instrumented version of functions
> - instrumented_version points to instrumented/original node
> - orig_decl holds original function declaration for instrumented nodes in case original node is removed
>
> IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals. It is used to have proper reachability analysis.
>
> When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function. Therefore new field appeared in cgraph_thunk_info to mark such thunks.
>
> Does it look OK?
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-04-16 Ilya Enkovich <ilya.enkovich@intel.com>
>
> * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
> field.
> (cgraph_node): Add instrumented_version, orig_decl and
> instrumentation_clone fields.
> (symtab_alias_target): Allow IPA_REF_CHKP reference.
> * cgraph.c (cgraph_remove_node): Fix instrumented_version
> of the referenced node if any.
> (dump_cgraph_node): Dump instrumentation_clone and
> instrumented_version fields.
> (verify_cgraph_node): Check correctness of IPA_REF_CHKP
> references and instrumentation thunks.
> * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
> reference.
> (cgraph_rebuild_references): Likewise.
> * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
> calling instrumneted function version.
> * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
> (ipa_ref): increase size of use field.
> * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
> * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
> thunk.add_pointer_bounds_args and orig_decl field.
> (lto_output_ref): Adjust to new ipa_ref::use field size.
> (input_overwrite_node): Read instrumentation_clone field.
> (input_node): Read thunk.add_pointer_bounds_args and orig_decl
> fields.
> (input_ref): Adjust to new ipa_ref::use field size.
> (input_cgraph_1): Compute instrumented_version fields and restore
> IDENTIFIER_TRANSPARENT_ALIAS chains.
> * lto-streamer.h (LTO_minor_version): Change minor version from
> 0 to 1.
> * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
> clone as address taken if the original one is address taken.
> (cgraph_externally_visible_p): Mark instrumented 'main' as
> externally visible.
> (function_and_variable_visibility): Filter instrumentation
> thunks.
>
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index be3661a..6210c68 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
> }
> for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
> i, ref); i++)
> - if (ref->use != IPA_REF_ALIAS)
> + if (ref->use == IPA_REF_CHKP)
> + ;
> + else if (ref->use != IPA_REF_ALIAS)
> {
> error ("Alias has non-alias reference");
> error_found = true;
Is there any checking you can/should be doing here? And I'm asking
because I'm pretty sure there's something you ought to be checking here :-)
There's a general desire for key datastructures to sanity check them as
much as possible.
> + /* If instrumentation_clone is 1 then instrumented_version points
> + to the original function used to make instrumented version.
> + Otherwise points to instrumented version of the function. */
> + struct cgraph_node *instrumented_version;
> + /* If instrumentation_clone is 1 then orig_decl is the original
> + function declaration. */
> + tree orig_decl;
So I don't see anything which checks these two invariants.
Mostly it looks good. I do want to look at it again once the
verification stuff is beefed up.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-23 13:47 ` Jeff Law
@ 2014-07-24 9:59 ` Ilya Enkovich
2014-07-24 12:05 ` Jan Hubicka
2014-09-03 19:32 ` Jeff Law
0 siblings, 2 replies; 11+ messages in thread
From: Ilya Enkovich @ 2014-07-24 9:59 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On 22 Jul 21:56, Jeff Law wrote:
> On 04/16/14 08:03, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch introduces changes in call graph for Pointer Bounds Checker.
> >
> >New fields instrumented_version, instrumentation_clone and orig_decl are added for cgraph_node:
> > - instrumentation_clone field is 1 for nodes created for instrumented version of functions
> > - instrumented_version points to instrumented/original node
> > - orig_decl holds original function declaration for instrumented nodes in case original node is removed
> >
> >IPA_REF_CHKP reference type is introduced for nodes to reference instrumented function versions from originals. It is used to have proper reachability analysis.
> >
> >When original function bodies are not needed anymore, functions are transformed into thunks having call edge to the instrumented function. Therefore new field appeared in cgraph_thunk_info to mark such thunks.
> >
> >Does it look OK?
> >
> >Bootstrapped and tested on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >gcc/
> >
> >2014-04-16 Ilya Enkovich <ilya.enkovich@intel.com>
> >
> > * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
> > field.
> > (cgraph_node): Add instrumented_version, orig_decl and
> > instrumentation_clone fields.
> > (symtab_alias_target): Allow IPA_REF_CHKP reference.
> > * cgraph.c (cgraph_remove_node): Fix instrumented_version
> > of the referenced node if any.
> > (dump_cgraph_node): Dump instrumentation_clone and
> > instrumented_version fields.
> > (verify_cgraph_node): Check correctness of IPA_REF_CHKP
> > references and instrumentation thunks.
> > * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
> > reference.
> > (cgraph_rebuild_references): Likewise.
> > * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
> > calling instrumneted function version.
> > * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
> > (ipa_ref): increase size of use field.
> > * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
> > * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
> > thunk.add_pointer_bounds_args and orig_decl field.
> > (lto_output_ref): Adjust to new ipa_ref::use field size.
> > (input_overwrite_node): Read instrumentation_clone field.
> > (input_node): Read thunk.add_pointer_bounds_args and orig_decl
> > fields.
> > (input_ref): Adjust to new ipa_ref::use field size.
> > (input_cgraph_1): Compute instrumented_version fields and restore
> > IDENTIFIER_TRANSPARENT_ALIAS chains.
> > * lto-streamer.h (LTO_minor_version): Change minor version from
> > 0 to 1.
> > * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
> > clone as address taken if the original one is address taken.
> > (cgraph_externally_visible_p): Mark instrumented 'main' as
> > externally visible.
> > (function_and_variable_visibility): Filter instrumentation
> > thunks.
> >
> >
> >diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> >index be3661a..6210c68 100644
> >--- a/gcc/cgraph.c
> >+++ b/gcc/cgraph.c
> >@@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
> > }
> > for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
> > i, ref); i++)
> >- if (ref->use != IPA_REF_ALIAS)
> >+ if (ref->use == IPA_REF_CHKP)
> >+ ;
> >+ else if (ref->use != IPA_REF_ALIAS)
> > {
> > error ("Alias has non-alias reference");
> > error_found = true;
> Is there any checking you can/should be doing here? And I'm
> asking because I'm pretty sure there's something you ought to be
> checking here :-)
>
> There's a general desire for key datastructures to sanity check them
> as much as possible.
Thanks for comments! I added additional check for chkp references. It is performed later because this piece of code is for aliases only.
>
> >+ /* If instrumentation_clone is 1 then instrumented_version points
> >+ to the original function used to make instrumented version.
> >+ Otherwise points to instrumented version of the function. */
> >+ struct cgraph_node *instrumented_version;
> >+ /* If instrumentation_clone is 1 then orig_decl is the original
> >+ function declaration. */
> >+ tree orig_decl;
> So I don't see anything which checks these two invariants.
>
> Mostly it looks good. I do want to look at it again once the
> verification stuff is beefed up.
>
>
> Jeff
Added checks for all new fields. Below is a new patch version.
Thanks,
Ilya
--
2014-07-24 Ilya Enkovich <ilya.enkovich@intel.com>
* cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
field.
(cgraph_node): Add instrumented_version, orig_decl and
instrumentation_clone fields.
(symtab_alias_target): Allow IPA_REF_CHKP reference.
* cgraph.c (cgraph_remove_node): Fix instrumented_version
of the referenced node if any.
(dump_cgraph_node): Dump instrumentation_clone and
instrumented_version fields.
(verify_cgraph_node): Check correctness of IPA_REF_CHKP
references and instrumentation thunks.
* cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
reference.
(cgraph_rebuild_references): Likewise.
* cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
calling instrumneted function version.
* ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
(ipa_ref): increase size of use field.
* ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
* lto-cgraph.c (lto_output_node): Output instrumentation_clone,
thunk.add_pointer_bounds_args and orig_decl field.
(lto_output_ref): Adjust to new ipa_ref::use field size.
(input_overwrite_node): Read instrumentation_clone field.
(input_node): Read thunk.add_pointer_bounds_args and orig_decl
fields.
(input_ref): Adjust to new ipa_ref::use field size.
(input_cgraph_1): Compute instrumented_version fields and restore
IDENTIFIER_TRANSPARENT_ALIAS chains.
* lto-streamer.h (LTO_minor_version): Change minor version from
0 to 1.
* ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
clone as address taken if the original one is address taken.
(cgraph_externally_visible_p): Mark instrumented 'main' as
externally visible.
(function_and_variable_visibility): Filter instrumentation
thunks.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index be3661a..cfd8708 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1828,6 +1828,12 @@ cgraph_remove_node (struct cgraph_node *node)
}
cgraph_n_nodes--;
+ if (node->instrumented_version)
+ {
+ node->instrumented_version->instrumented_version = NULL;
+ node->instrumented_version = NULL;
+ }
+
/* Clear out the node to NULL all pointers and add the node to the free
list. */
memset (node, 0, sizeof (*node));
@@ -2070,6 +2076,11 @@ dump_cgraph_node (FILE *f, struct cgraph_node *node)
if (indirect_calls_count)
fprintf (f, " Has %i outgoing edges for indirect calls.\n",
indirect_calls_count);
+
+ if (node->instrumentation_clone)
+ fprintf (f, " Is instrumented version.\n");
+ else if (node->instrumented_version)
+ fprintf (f, " Has instrumented version.\n");
}
@@ -2850,7 +2861,9 @@ verify_cgraph_node (struct cgraph_node *node)
}
for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
i, ref); i++)
- if (ref->use != IPA_REF_ALIAS)
+ if (ref->use == IPA_REF_CHKP)
+ ;
+ else if (ref->use != IPA_REF_ALIAS)
{
error ("Alias has non-alias reference");
error_found = true;
@@ -2868,6 +2881,65 @@ verify_cgraph_node (struct cgraph_node *node)
error_found = true;
}
}
+
+ /* Check instrumented version reference. */
+ if (node->instrumented_version
+ && node->instrumented_version->instrumented_version != node)
+ {
+ error ("Instrumentation clone does not reference original node");
+ error_found = true;
+ }
+
+ /* Cannot have orig_decl for not instrumented nodes. */
+ if (!node->instrumentation_clone && node->orig_decl)
+ {
+ error ("Not instrumented node has non-NULL original declaration");
+ error_found = true;
+ }
+
+ /* If original not instrumented node still exists then we may check
+ original declaration is set properly. */
+ if (node->instrumented_version
+ && node->orig_decl
+ && node->orig_decl != node->instrumented_version->decl)
+ {
+ error ("Instrumented node has wrong original declaration");
+ error_found = true;
+ }
+
+ /* Check all nodes have chkp reference to their instrumented versions. */
+ if (node->analyzed
+ && node->instrumented_version
+ && !node->instrumentation_clone)
+ {
+ bool ref_found = false;
+ int i;
+ struct ipa_ref *ref;
+
+ for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list,
+ i, ref); i++)
+ if (ref->use == IPA_REF_CHKP)
+ {
+ if (ref_found)
+ {
+ error ("Node has more than one chkp reference");
+ error_found = true;
+ }
+ if (ref->referred != node->instrumented_version)
+ {
+ error ("Wrong node is referenced with chkp reference");
+ error_found = true;
+ }
+ ref_found = true;
+ }
+
+ if (!ref_found)
+ {
+ error ("Analyzed node has no reference to instrumented version");
+ error_found = true;
+ }
+ }
+
if (node->analyzed && node->thunk.thunk_p)
{
if (!node->callees)
@@ -2885,6 +2957,12 @@ verify_cgraph_node (struct cgraph_node *node)
error ("Thunk is not supposed to have body");
error_found = true;
}
+ if (node->thunk.add_pointer_bounds_args
+ && node->callees->callee != node->instrumented_version)
+ {
+ error ("Instrumentation thunk has wrong edge callee");
+ error_found = true;
+ }
}
else if (node->analyzed && gimple_has_body_p (node->decl)
&& !TREE_ASM_WRITTEN (node->decl)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a6a51cf..5e702a7 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
tree alias;
bool this_adjusting;
bool virtual_offset_p;
+ bool add_pointer_bounds_args;
/* Set to true when alias node is thunk. */
bool thunk_p;
};
@@ -373,6 +374,13 @@ public:
struct cgraph_node *prev_sibling_clone;
struct cgraph_node *clones;
struct cgraph_node *clone_of;
+ /* If instrumentation_clone is 1 then instrumented_version points
+ to the original function used to make instrumented version.
+ Otherwise points to instrumented version of the function. */
+ struct cgraph_node *instrumented_version;
+ /* If instrumentation_clone is 1 then orig_decl is the original
+ function declaration. */
+ tree orig_decl;
/* For functions with many calls sites it holds map from call expression
to the edge to speed up cgraph_edge function. */
htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
@@ -433,6 +441,9 @@ public:
/* True if this decl calls a COMDAT-local function. This is set up in
compute_inline_parameters and inline_call. */
unsigned calls_comdat_local : 1;
+ /* True when function is clone created for Pointer Bounds Checker
+ instrumentation. */
+ unsigned instrumentation_clone : 1;
};
@@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
{
struct ipa_ref *ref;
ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
+ if (ref->use == IPA_REF_CHKP)
+ ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
gcc_checking_assert (ref->use == IPA_REF_ALIAS);
return ref->referred;
}
diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index 19961e2..a2b2106 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
record_eh_tables (node, cfun);
gcc_assert (!node->global.inlined_to);
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
+
return 0;
}
@@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
ipa_record_stmt_references (node, gsi_stmt (gsi));
}
record_eh_tables (node, cfun);
+
+
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
}
namespace {
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 06283fc..ceb4060 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
struct ipa_ref *ref;
for (e = node->callers; e;)
- if (e->caller->thunk.thunk_p)
+ if (e->caller->thunk.thunk_p
+ && !e->caller->thunk.add_pointer_bounds_args)
{
struct cgraph_node *thunk = e->caller;
diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
index 6aa41e6..3a055d9 100644
--- a/gcc/ipa-ref.c
+++ b/gcc/ipa-ref.c
@@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
#include "cgraph.h"
#include "ipa-utils.h"
-static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
+static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
/* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index 4ce5f8d..d0df0bf 100644
--- a/gcc/ipa-ref.h
+++ b/gcc/ipa-ref.h
@@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
IPA_REF_LOAD,
IPA_REF_STORE,
IPA_REF_ADDR,
- IPA_REF_ALIAS
+ IPA_REF_ALIAS,
+ IPA_REF_CHKP
};
/* Record of reference in callgraph or varpool. */
@@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
gimple stmt;
unsigned int lto_stmt_uid;
unsigned int referred_index;
- ENUM_BITFIELD (ipa_ref_use) use:2;
+ ENUM_BITFIELD (ipa_ref_use) use:3;
unsigned int speculative:1;
};
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 5ab3aed..1d7fa35 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
cgraph_node_remove_callees (node);
ipa_remove_all_references (&node->ref_list);
changed = true;
+ if (node->thunk.thunk_p
+ && node->thunk.add_pointer_bounds_args)
+ {
+ node->thunk.thunk_p = false;
+ node->thunk.add_pointer_bounds_args = false;
+ }
}
}
else
@@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
if (node->address_taken
&& !node->used_from_other_partition)
{
- if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
+ if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
+ && (!node->instrumentation_clone
+ || !node->instrumented_version
+ || !node->instrumented_version->address_taken))
{
if (file)
fprintf (file, " %s", node->name ());
@@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
if (MAIN_NAME_P (DECL_NAME (node->decl)))
return true;
+ if (node->instrumentation_clone
+ && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
+ return true;
+
return false;
}
@@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
}
if (node->thunk.thunk_p
+ && !node->thunk.add_pointer_bounds_args
&& TREE_PUBLIC (node->decl))
{
struct cgraph_node *decl_node = node;
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 999ce3d..58105f0 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
bp_pack_enum (&bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN, node->resolution);
+ bp_pack_value (&bp, node->instrumentation_clone, 1);
streamer_write_bitpack (&bp);
if (node->thunk.thunk_p && !boundary_p)
@@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
streamer_write_uhwi_stream
(ob->main_stream,
1 + (node->thunk.this_adjusting != 0) * 2
- + (node->thunk.virtual_offset_p != 0) * 4);
+ + (node->thunk.virtual_offset_p != 0) * 4
+ + (node->thunk.add_pointer_bounds_args != 0) * 8);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
}
streamer_write_hwi_stream (ob->main_stream, node->profile_id);
+
+ if (node->instrumentation_clone)
+ lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
}
/* Output the varpool NODE to OB.
@@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
struct cgraph_node *node;
bp = bitpack_create (ob->main_stream);
- bp_pack_value (&bp, ref->use, 2);
+ bp_pack_value (&bp, ref->use, 3);
bp_pack_value (&bp, ref->speculative, 1);
streamer_write_bitpack (&bp);
nref = lto_symtab_encoder_lookup (encoder, ref->referred);
@@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
node->thunk.thunk_p = bp_unpack_value (bp, 1);
node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN);
+ node->instrumentation_clone = bp_unpack_value (bp, 1);
gcc_assert (flag_ltrans
|| (!node->in_other_partition
&& !node->used_from_other_partition));
@@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
node->thunk.this_adjusting = (type & 2);
node->thunk.virtual_value = virtual_value;
node->thunk.virtual_offset_p = (type & 4);
+ node->thunk.add_pointer_bounds_args = (type & 8);
}
if (node->alias && !node->analyzed && node->weakref)
node->alias_target = get_alias_symbol (node->decl);
node->profile_id = streamer_read_hwi (ib);
+
+ if (node->instrumentation_clone)
+ {
+ decl_index = streamer_read_uhwi (ib);
+ fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
+ node->orig_decl = fn_decl;
+ }
+
return node;
}
@@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
struct ipa_ref *ref;
bp = streamer_read_bitpack (ib);
- use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
+ use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
node = nodes[streamer_read_hwi (ib)];
ref = ipa_record_reference (referring_node, node, use, NULL);
@@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
else
cnode->global.inlined_to = NULL;
+
+ /* Compute instrumented_version. */
+ if (cnode->instrumentation_clone)
+ {
+ gcc_assert (cnode->orig_decl);
+
+ cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
+ if (cnode->instrumented_version)
+ cnode->instrumented_version->instrumented_version = cnode;
+
+ /* Restore decl names reference. */
+ if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
+ && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
+ TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
+ = DECL_ASSEMBLER_NAME (cnode->orig_decl);
+ }
}
ref = (int) (intptr_t) node->same_comdat_group;
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 51b1903..62a5fe0 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
#define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
#define LTO_major_version 3
-#define LTO_minor_version 0
+#define LTO_minor_version 1
typedef unsigned char lto_decl_flags_t;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-24 9:59 ` Ilya Enkovich
@ 2014-07-24 12:05 ` Jan Hubicka
2014-07-24 13:19 ` Ilya Enkovich
2014-09-03 19:32 ` Jeff Law
1 sibling, 1 reply; 11+ messages in thread
From: Jan Hubicka @ 2014-07-24 12:05 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: Jeff Law, gcc-patches
Hello,
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index a6a51cf..5e702a7 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
> tree alias;
> bool this_adjusting;
> bool virtual_offset_p;
> + bool add_pointer_bounds_args;
> /* Set to true when alias node is thunk. */
> bool thunk_p;
> };
> @@ -373,6 +374,13 @@ public:
> struct cgraph_node *prev_sibling_clone;
> struct cgraph_node *clones;
> struct cgraph_node *clone_of;
> + /* If instrumentation_clone is 1 then instrumented_version points
> + to the original function used to make instrumented version.
> + Otherwise points to instrumented version of the function. */
> + struct cgraph_node *instrumented_version;
> + /* If instrumentation_clone is 1 then orig_decl is the original
> + function declaration. */
> + tree orig_decl;
So the patch is introducing yet another notion of clone (in addition to existing virtual clones
and function versions used by ifun) and you add a new type of reference (CHKP) to link the
original and the clone.
Why do you need to link things in 3 different ways? (i.e. instrumented_version points to the
same place as CHKP and as orig_decl, right?).
I would preffer if this can be put into the existing clone mechanizm. The virtual clones can
have quite generic transformations done on them and the do perform all the necessary links
back and forth.
I will look into the rest of changes, is there some overview?
Honza
> /* For functions with many calls sites it holds map from call expression
> to the edge to speed up cgraph_edge function. */
> htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
> @@ -433,6 +441,9 @@ public:
> /* True if this decl calls a COMDAT-local function. This is set up in
> compute_inline_parameters and inline_call. */
> unsigned calls_comdat_local : 1;
> + /* True when function is clone created for Pointer Bounds Checker
> + instrumentation. */
> + unsigned instrumentation_clone : 1;
> };
>
>
> @@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
> {
> struct ipa_ref *ref;
> ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
> + if (ref->use == IPA_REF_CHKP)
> + ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
> gcc_checking_assert (ref->use == IPA_REF_ALIAS);
> return ref->referred;
> }
> diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
> index 19961e2..a2b2106 100644
> --- a/gcc/cgraphbuild.c
> +++ b/gcc/cgraphbuild.c
> @@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
> record_eh_tables (node, cfun);
> gcc_assert (!node->global.inlined_to);
>
> + if (node->instrumented_version
> + && !node->instrumentation_clone)
> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> +
> return 0;
> }
>
> @@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
> ipa_record_stmt_references (node, gsi_stmt (gsi));
> }
> record_eh_tables (node, cfun);
> +
> +
> + if (node->instrumented_version
> + && !node->instrumentation_clone)
> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> }
>
> namespace {
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 06283fc..ceb4060 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
> struct ipa_ref *ref;
>
> for (e = node->callers; e;)
> - if (e->caller->thunk.thunk_p)
> + if (e->caller->thunk.thunk_p
> + && !e->caller->thunk.add_pointer_bounds_args)
> {
> struct cgraph_node *thunk = e->caller;
>
> diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
> index 6aa41e6..3a055d9 100644
> --- a/gcc/ipa-ref.c
> +++ b/gcc/ipa-ref.c
> @@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
> #include "cgraph.h"
> #include "ipa-utils.h"
>
> -static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
> +static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
>
> /* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
> to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
> diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
> index 4ce5f8d..d0df0bf 100644
> --- a/gcc/ipa-ref.h
> +++ b/gcc/ipa-ref.h
> @@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
> IPA_REF_LOAD,
> IPA_REF_STORE,
> IPA_REF_ADDR,
> - IPA_REF_ALIAS
> + IPA_REF_ALIAS,
> + IPA_REF_CHKP
> };
>
> /* Record of reference in callgraph or varpool. */
> @@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
> gimple stmt;
> unsigned int lto_stmt_uid;
> unsigned int referred_index;
> - ENUM_BITFIELD (ipa_ref_use) use:2;
> + ENUM_BITFIELD (ipa_ref_use) use:3;
> unsigned int speculative:1;
> };
>
> diff --git a/gcc/ipa.c b/gcc/ipa.c
> index 5ab3aed..1d7fa35 100644
> --- a/gcc/ipa.c
> +++ b/gcc/ipa.c
> @@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
> cgraph_node_remove_callees (node);
> ipa_remove_all_references (&node->ref_list);
> changed = true;
> + if (node->thunk.thunk_p
> + && node->thunk.add_pointer_bounds_args)
> + {
> + node->thunk.thunk_p = false;
> + node->thunk.add_pointer_bounds_args = false;
> + }
> }
> }
> else
> @@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
> if (node->address_taken
> && !node->used_from_other_partition)
> {
> - if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
> + if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
> + && (!node->instrumentation_clone
> + || !node->instrumented_version
> + || !node->instrumented_version->address_taken))
> {
> if (file)
> fprintf (file, " %s", node->name ());
> @@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
> if (MAIN_NAME_P (DECL_NAME (node->decl)))
> return true;
>
> + if (node->instrumentation_clone
> + && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
> + return true;
> +
> return false;
> }
>
> @@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
> }
>
> if (node->thunk.thunk_p
> + && !node->thunk.add_pointer_bounds_args
> && TREE_PUBLIC (node->decl))
> {
> struct cgraph_node *decl_node = node;
> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> index 999ce3d..58105f0 100644
> --- a/gcc/lto-cgraph.c
> +++ b/gcc/lto-cgraph.c
> @@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
> bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
> bp_pack_enum (&bp, ld_plugin_symbol_resolution,
> LDPR_NUM_KNOWN, node->resolution);
> + bp_pack_value (&bp, node->instrumentation_clone, 1);
> streamer_write_bitpack (&bp);
>
> if (node->thunk.thunk_p && !boundary_p)
> @@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
> streamer_write_uhwi_stream
> (ob->main_stream,
> 1 + (node->thunk.this_adjusting != 0) * 2
> - + (node->thunk.virtual_offset_p != 0) * 4);
> + + (node->thunk.virtual_offset_p != 0) * 4
> + + (node->thunk.add_pointer_bounds_args != 0) * 8);
> streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
> streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
> }
> streamer_write_hwi_stream (ob->main_stream, node->profile_id);
> +
> + if (node->instrumentation_clone)
> + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
> }
>
> /* Output the varpool NODE to OB.
> @@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
> struct cgraph_node *node;
>
> bp = bitpack_create (ob->main_stream);
> - bp_pack_value (&bp, ref->use, 2);
> + bp_pack_value (&bp, ref->use, 3);
> bp_pack_value (&bp, ref->speculative, 1);
> streamer_write_bitpack (&bp);
> nref = lto_symtab_encoder_lookup (encoder, ref->referred);
> @@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
> node->thunk.thunk_p = bp_unpack_value (bp, 1);
> node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
> LDPR_NUM_KNOWN);
> + node->instrumentation_clone = bp_unpack_value (bp, 1);
> gcc_assert (flag_ltrans
> || (!node->in_other_partition
> && !node->used_from_other_partition));
> @@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
> node->thunk.this_adjusting = (type & 2);
> node->thunk.virtual_value = virtual_value;
> node->thunk.virtual_offset_p = (type & 4);
> + node->thunk.add_pointer_bounds_args = (type & 8);
> }
> if (node->alias && !node->analyzed && node->weakref)
> node->alias_target = get_alias_symbol (node->decl);
> node->profile_id = streamer_read_hwi (ib);
> +
> + if (node->instrumentation_clone)
> + {
> + decl_index = streamer_read_uhwi (ib);
> + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
> + node->orig_decl = fn_decl;
> + }
> +
> return node;
> }
>
> @@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
> struct ipa_ref *ref;
>
> bp = streamer_read_bitpack (ib);
> - use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
> + use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
> speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
> node = nodes[streamer_read_hwi (ib)];
> ref = ipa_record_reference (referring_node, node, use, NULL);
> @@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
> cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
> else
> cnode->global.inlined_to = NULL;
> +
> + /* Compute instrumented_version. */
> + if (cnode->instrumentation_clone)
> + {
> + gcc_assert (cnode->orig_decl);
> +
> + cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
> + if (cnode->instrumented_version)
> + cnode->instrumented_version->instrumented_version = cnode;
> +
> + /* Restore decl names reference. */
> + if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
> + && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
> + TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
> + = DECL_ASSEMBLER_NAME (cnode->orig_decl);
> + }
> }
>
> ref = (int) (intptr_t) node->same_comdat_group;
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 51b1903..62a5fe0 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
> #define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
>
> #define LTO_major_version 3
> -#define LTO_minor_version 0
> +#define LTO_minor_version 1
>
> typedef unsigned char lto_decl_flags_t;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-24 12:05 ` Jan Hubicka
@ 2014-07-24 13:19 ` Ilya Enkovich
2014-07-24 13:52 ` Jan Hubicka
0 siblings, 1 reply; 11+ messages in thread
From: Ilya Enkovich @ 2014-07-24 13:19 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jeff Law, gcc-patches
2014-07-24 15:38 GMT+04:00 Jan Hubicka <hubicka@ucw.cz>:
> Hello,
>
>> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
>> index a6a51cf..5e702a7 100644
>> --- a/gcc/cgraph.h
>> +++ b/gcc/cgraph.h
>> @@ -191,6 +191,7 @@ struct GTY(()) cgraph_thunk_info {
>> tree alias;
>> bool this_adjusting;
>> bool virtual_offset_p;
>> + bool add_pointer_bounds_args;
>> /* Set to true when alias node is thunk. */
>> bool thunk_p;
>> };
>> @@ -373,6 +374,13 @@ public:
>> struct cgraph_node *prev_sibling_clone;
>> struct cgraph_node *clones;
>> struct cgraph_node *clone_of;
>> + /* If instrumentation_clone is 1 then instrumented_version points
>> + to the original function used to make instrumented version.
>> + Otherwise points to instrumented version of the function. */
>> + struct cgraph_node *instrumented_version;
>> + /* If instrumentation_clone is 1 then orig_decl is the original
>> + function declaration. */
>> + tree orig_decl;
>
> So the patch is introducing yet another notion of clone (in addition to existing virtual clones
> and function versions used by ifun) and you add a new type of reference (CHKP) to link the
> original and the clone.
>
> Why do you need to link things in 3 different ways? (i.e. instrumented_version points to the
> same place as CHKP and as orig_decl, right?).
CHKP reference is required to have reachability algorithms working
correctly and not removing required instrumented nodes. References
are rebuilt time to time and instrumented_version is used to rebuild
CHKP reference. orig_decl is required because original function node
may be removed as unreachable.
>
> I would preffer if this can be put into the existing clone mechanizm. The virtual clones can
> have quite generic transformations done on them and the do perform all the necessary links
> back and forth.
I suppose virtual clones are useful when we may delay their
materialization, i.e. for IPA passes. For checker we have
instrumentation almost immediately following clone creation.
Instrumentation is a GIMPLE pass and we have to materialize clones to
have bodies to instrument. After materialization there is no link to
original node anymore and it means we would still require all new
fields in cgraph_node structure.
>
> I will look into the rest of changes, is there some overview?
I have a short overview of how it works on a wiki page:
https://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler#Instrumentation_clones
Thanks,
Ilya
>
> Honza
>
>
>> /* For functions with many calls sites it holds map from call expression
>> to the edge to speed up cgraph_edge function. */
>> htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
>> @@ -433,6 +441,9 @@ public:
>> /* True if this decl calls a COMDAT-local function. This is set up in
>> compute_inline_parameters and inline_call. */
>> unsigned calls_comdat_local : 1;
>> + /* True when function is clone created for Pointer Bounds Checker
>> + instrumentation. */
>> + unsigned instrumentation_clone : 1;
>> };
>>
>>
>> @@ -1412,6 +1423,8 @@ symtab_alias_target (symtab_node *n)
>> {
>> struct ipa_ref *ref;
>> ipa_ref_list_reference_iterate (&n->ref_list, 0, ref);
>> + if (ref->use == IPA_REF_CHKP)
>> + ipa_ref_list_reference_iterate (&n->ref_list, 1, ref);
>> gcc_checking_assert (ref->use == IPA_REF_ALIAS);
>> return ref->referred;
>> }
>> diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
>> index 19961e2..a2b2106 100644
>> --- a/gcc/cgraphbuild.c
>> +++ b/gcc/cgraphbuild.c
>> @@ -481,6 +481,10 @@ rebuild_cgraph_edges (void)
>> record_eh_tables (node, cfun);
>> gcc_assert (!node->global.inlined_to);
>>
>> + if (node->instrumented_version
>> + && !node->instrumentation_clone)
>> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
>> +
>> return 0;
>> }
>>
>> @@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
>> ipa_record_stmt_references (node, gsi_stmt (gsi));
>> }
>> record_eh_tables (node, cfun);
>> +
>> +
>> + if (node->instrumented_version
>> + && !node->instrumentation_clone)
>> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
>> }
>>
>> namespace {
>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index 06283fc..ceb4060 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -1702,7 +1702,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
>> struct ipa_ref *ref;
>>
>> for (e = node->callers; e;)
>> - if (e->caller->thunk.thunk_p)
>> + if (e->caller->thunk.thunk_p
>> + && !e->caller->thunk.add_pointer_bounds_args)
>> {
>> struct cgraph_node *thunk = e->caller;
>>
>> diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
>> index 6aa41e6..3a055d9 100644
>> --- a/gcc/ipa-ref.c
>> +++ b/gcc/ipa-ref.c
>> @@ -27,7 +27,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "cgraph.h"
>> #include "ipa-utils.h"
>>
>> -static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
>> +static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
>>
>> /* Return ipa reference from REFERING_NODE or REFERING_VARPOOL_NODE
>> to REFERED_NODE or REFERED_VARPOOL_NODE. USE_TYPE specify type
>> diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
>> index 4ce5f8d..d0df0bf 100644
>> --- a/gcc/ipa-ref.h
>> +++ b/gcc/ipa-ref.h
>> @@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
>> IPA_REF_LOAD,
>> IPA_REF_STORE,
>> IPA_REF_ADDR,
>> - IPA_REF_ALIAS
>> + IPA_REF_ALIAS,
>> + IPA_REF_CHKP
>> };
>>
>> /* Record of reference in callgraph or varpool. */
>> @@ -40,7 +41,7 @@ struct GTY(()) ipa_ref
>> gimple stmt;
>> unsigned int lto_stmt_uid;
>> unsigned int referred_index;
>> - ENUM_BITFIELD (ipa_ref_use) use:2;
>> + ENUM_BITFIELD (ipa_ref_use) use:3;
>> unsigned int speculative:1;
>> };
>>
>> diff --git a/gcc/ipa.c b/gcc/ipa.c
>> index 5ab3aed..1d7fa35 100644
>> --- a/gcc/ipa.c
>> +++ b/gcc/ipa.c
>> @@ -508,6 +508,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
>> cgraph_node_remove_callees (node);
>> ipa_remove_all_references (&node->ref_list);
>> changed = true;
>> + if (node->thunk.thunk_p
>> + && node->thunk.add_pointer_bounds_args)
>> + {
>> + node->thunk.thunk_p = false;
>> + node->thunk.add_pointer_bounds_args = false;
>> + }
>> }
>> }
>> else
>> @@ -583,7 +589,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
>> if (node->address_taken
>> && !node->used_from_other_partition)
>> {
>> - if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true))
>> + if (!cgraph_for_node_and_aliases (node, has_addr_references_p, NULL, true)
>> + && (!node->instrumentation_clone
>> + || !node->instrumented_version
>> + || !node->instrumented_version->address_taken))
>> {
>> if (file)
>> fprintf (file, " %s", node->name ());
>> @@ -814,6 +823,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
>> if (MAIN_NAME_P (DECL_NAME (node->decl)))
>> return true;
>>
>> + if (node->instrumentation_clone
>> + && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
>> + return true;
>> +
>> return false;
>> }
>>
>> @@ -1016,6 +1029,7 @@ function_and_variable_visibility (bool whole_program)
>> }
>>
>> if (node->thunk.thunk_p
>> + && !node->thunk.add_pointer_bounds_args
>> && TREE_PUBLIC (node->decl))
>> {
>> struct cgraph_node *decl_node = node;
>> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
>> index 999ce3d..58105f0 100644
>> --- a/gcc/lto-cgraph.c
>> +++ b/gcc/lto-cgraph.c
>> @@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>> bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
>> bp_pack_enum (&bp, ld_plugin_symbol_resolution,
>> LDPR_NUM_KNOWN, node->resolution);
>> + bp_pack_value (&bp, node->instrumentation_clone, 1);
>> streamer_write_bitpack (&bp);
>>
>> if (node->thunk.thunk_p && !boundary_p)
>> @@ -533,11 +534,15 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>> streamer_write_uhwi_stream
>> (ob->main_stream,
>> 1 + (node->thunk.this_adjusting != 0) * 2
>> - + (node->thunk.virtual_offset_p != 0) * 4);
>> + + (node->thunk.virtual_offset_p != 0) * 4
>> + + (node->thunk.add_pointer_bounds_args != 0) * 8);
>> streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
>> streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
>> }
>> streamer_write_hwi_stream (ob->main_stream, node->profile_id);
>> +
>> + if (node->instrumentation_clone)
>> + lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
>> }
>>
>> /* Output the varpool NODE to OB.
>> @@ -613,7 +618,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
>> struct cgraph_node *node;
>>
>> bp = bitpack_create (ob->main_stream);
>> - bp_pack_value (&bp, ref->use, 2);
>> + bp_pack_value (&bp, ref->use, 3);
>> bp_pack_value (&bp, ref->speculative, 1);
>> streamer_write_bitpack (&bp);
>> nref = lto_symtab_encoder_lookup (encoder, ref->referred);
>> @@ -1002,6 +1007,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
>> node->thunk.thunk_p = bp_unpack_value (bp, 1);
>> node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
>> LDPR_NUM_KNOWN);
>> + node->instrumentation_clone = bp_unpack_value (bp, 1);
>> gcc_assert (flag_ltrans
>> || (!node->in_other_partition
>> && !node->used_from_other_partition));
>> @@ -1112,10 +1118,19 @@ input_node (struct lto_file_decl_data *file_data,
>> node->thunk.this_adjusting = (type & 2);
>> node->thunk.virtual_value = virtual_value;
>> node->thunk.virtual_offset_p = (type & 4);
>> + node->thunk.add_pointer_bounds_args = (type & 8);
>> }
>> if (node->alias && !node->analyzed && node->weakref)
>> node->alias_target = get_alias_symbol (node->decl);
>> node->profile_id = streamer_read_hwi (ib);
>> +
>> + if (node->instrumentation_clone)
>> + {
>> + decl_index = streamer_read_uhwi (ib);
>> + fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
>> + node->orig_decl = fn_decl;
>> + }
>> +
>> return node;
>> }
>>
>> @@ -1196,7 +1211,7 @@ input_ref (struct lto_input_block *ib,
>> struct ipa_ref *ref;
>>
>> bp = streamer_read_bitpack (ib);
>> - use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
>> + use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
>> speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
>> node = nodes[streamer_read_hwi (ib)];
>> ref = ipa_record_reference (referring_node, node, use, NULL);
>> @@ -1337,6 +1352,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
>> cgraph (node)->global.inlined_to = cgraph (nodes[ref]);
>> else
>> cnode->global.inlined_to = NULL;
>> +
>> + /* Compute instrumented_version. */
>> + if (cnode->instrumentation_clone)
>> + {
>> + gcc_assert (cnode->orig_decl);
>> +
>> + cnode->instrumented_version = cgraph_get_node (cnode->orig_decl);
>> + if (cnode->instrumented_version)
>> + cnode->instrumented_version->instrumented_version = cnode;
>> +
>> + /* Restore decl names reference. */
>> + if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
>> + && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
>> + TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
>> + = DECL_ASSEMBLER_NAME (cnode->orig_decl);
>> + }
>> }
>>
>> ref = (int) (intptr_t) node->same_comdat_group;
>> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
>> index 51b1903..62a5fe0 100644
>> --- a/gcc/lto-streamer.h
>> +++ b/gcc/lto-streamer.h
>> @@ -141,7 +141,7 @@ along with GCC; see the file COPYING3. If not see
>> #define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
>>
>> #define LTO_major_version 3
>> -#define LTO_minor_version 0
>> +#define LTO_minor_version 1
>>
>> typedef unsigned char lto_decl_flags_t;
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-24 13:19 ` Ilya Enkovich
@ 2014-07-24 13:52 ` Jan Hubicka
2014-07-25 11:20 ` Ilya Enkovich
0 siblings, 1 reply; 11+ messages in thread
From: Jan Hubicka @ 2014-07-24 13:52 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: Jan Hubicka, Jeff Law, gcc-patches
> > So the patch is introducing yet another notion of clone (in addition to existing virtual clones
> > and function versions used by ifun) and you add a new type of reference (CHKP) to link the
> > original and the clone.
> >
> > Why do you need to link things in 3 different ways? (i.e. instrumented_version points to the
> > same place as CHKP and as orig_decl, right?).
>
> CHKP reference is required to have reachability algorithms working
> correctly and not removing required instrumented nodes. References
> are rebuilt time to time and instrumented_version is used to rebuild
> CHKP reference. orig_decl is required because original function node
> may be removed as unreachable.
>
> >
> > I would preffer if this can be put into the existing clone mechanizm. The virtual clones can
> > have quite generic transformations done on them and the do perform all the necessary links
> > back and forth.
>
> I suppose virtual clones are useful when we may delay their
> materialization, i.e. for IPA passes. For checker we have
> instrumentation almost immediately following clone creation.
> Instrumentation is a GIMPLE pass and we have to materialize clones to
> have bodies to instrument. After materialization there is no link to
> original node anymore and it means we would still require all new
> fields in cgraph_node structure.
>
> >
> > I will look into the rest of changes, is there some overview?
>
> I have a short overview of how it works on a wiki page:
> https://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler#Instrumentation_clones
Thanks, I will take a deeper look. I am just somewhat concerned that you seem
to be duplicating a lot of logic that is already present in the other clonning
schemes we have (i.e. arranging sane partitining, keeping clones linked with
their original etc). We may want to generalize current mechanizm rather than
implementing similar in parallel...
Sorry for ignoring the patches so long - I seem to have missed my CC in original
thread. I would welcome if you CC hubicka@ucw.cz for cgraph/ipa related patches.
Honza
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-24 13:52 ` Jan Hubicka
@ 2014-07-25 11:20 ` Ilya Enkovich
0 siblings, 0 replies; 11+ messages in thread
From: Ilya Enkovich @ 2014-07-25 11:20 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jeff Law, gcc-patches
2014-07-24 17:41 GMT+04:00 Jan Hubicka <hubicka@ucw.cz>:
>> > So the patch is introducing yet another notion of clone (in addition to existing virtual clones
>> > and function versions used by ifun) and you add a new type of reference (CHKP) to link the
>> > original and the clone.
>> >
>> > Why do you need to link things in 3 different ways? (i.e. instrumented_version points to the
>> > same place as CHKP and as orig_decl, right?).
>>
>> CHKP reference is required to have reachability algorithms working
>> correctly and not removing required instrumented nodes. References
>> are rebuilt time to time and instrumented_version is used to rebuild
>> CHKP reference. orig_decl is required because original function node
>> may be removed as unreachable.
>>
>> >
>> > I would preffer if this can be put into the existing clone mechanizm. The virtual clones can
>> > have quite generic transformations done on them and the do perform all the necessary links
>> > back and forth.
>>
>> I suppose virtual clones are useful when we may delay their
>> materialization, i.e. for IPA passes. For checker we have
>> instrumentation almost immediately following clone creation.
>> Instrumentation is a GIMPLE pass and we have to materialize clones to
>> have bodies to instrument. After materialization there is no link to
>> original node anymore and it means we would still require all new
>> fields in cgraph_node structure.
>>
>> >
>> > I will look into the rest of changes, is there some overview?
>>
>> I have a short overview of how it works on a wiki page:
>> https://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler#Instrumentation_clones
>
> Thanks, I will take a deeper look. I am just somewhat concerned that you seem
> to be duplicating a lot of logic that is already present in the other clonning
> schemes we have (i.e. arranging sane partitining, keeping clones linked with
> their original etc). We may want to generalize current mechanizm rather than
> implementing similar in parallel...
Thanks for looking into this! I looked into clones mechanism before
introducing instrumentation clones and did not see how I can re-use
it. Probably you may see if it can be easily adopted.
Thanks,
Ilya
>
> Sorry for ignoring the patches so long - I seem to have missed my CC in original
> thread. I would welcome if you CC hubicka@ucw.cz for cgraph/ipa related patches.
>
> Honza
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-07-24 9:59 ` Ilya Enkovich
2014-07-24 12:05 ` Jan Hubicka
@ 2014-09-03 19:32 ` Jeff Law
2014-09-15 7:51 ` Ilya Enkovich
1 sibling, 1 reply; 11+ messages in thread
From: Jeff Law @ 2014-09-03 19:32 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches
On 07/24/14 03:59, Ilya Enkovich wrote:
> --
> 2014-07-24 Ilya Enkovich <ilya.enkovich@intel.com>
>
> * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
> field.
> (cgraph_node): Add instrumented_version, orig_decl and
> instrumentation_clone fields.
> (symtab_alias_target): Allow IPA_REF_CHKP reference.
> * cgraph.c (cgraph_remove_node): Fix instrumented_version
> of the referenced node if any.
> (dump_cgraph_node): Dump instrumentation_clone and
> instrumented_version fields.
> (verify_cgraph_node): Check correctness of IPA_REF_CHKP
> references and instrumentation thunks.
> * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
> reference.
> (cgraph_rebuild_references): Likewise.
> * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
> calling instrumneted function version.
> * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
> (ipa_ref): increase size of use field.
> * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
> * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
> thunk.add_pointer_bounds_args and orig_decl field.
> (lto_output_ref): Adjust to new ipa_ref::use field size.
> (input_overwrite_node): Read instrumentation_clone field.
> (input_node): Read thunk.add_pointer_bounds_args and orig_decl
> fields.
> (input_ref): Adjust to new ipa_ref::use field size.
> (input_cgraph_1): Compute instrumented_version fields and restore
> IDENTIFIER_TRANSPARENT_ALIAS chains.
> * lto-streamer.h (LTO_minor_version): Change minor version from
> 0 to 1.
> * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
> clone as address taken if the original one is address taken.
> (cgraph_externally_visible_p): Mark instrumented 'main' as
> externally visible.
> (function_and_variable_visibility): Filter instrumentation
> thunks.
Thanks for adding the additional checking.
> @@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
> ipa_record_stmt_references (node, gsi_stmt (gsi));
> }
> record_eh_tables (node, cfun);
> +
> +
> + if (node->instrumented_version
> + && !node->instrumentation_clone)
> + ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> }
Trivial nit here -- just one vertical space here.
With that nit fixed, this is OK.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, Pointer Bounds Checker 9/x] Cgraph extension
2014-09-03 19:32 ` Jeff Law
@ 2014-09-15 7:51 ` Ilya Enkovich
0 siblings, 0 replies; 11+ messages in thread
From: Ilya Enkovich @ 2014-09-15 7:51 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On 03 Sep 13:32, Jeff Law wrote:
> On 07/24/14 03:59, Ilya Enkovich wrote:
> >--
> >2014-07-24 Ilya Enkovich <ilya.enkovich@intel.com>
> >
> > * cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
> > field.
> > (cgraph_node): Add instrumented_version, orig_decl and
> > instrumentation_clone fields.
> > (symtab_alias_target): Allow IPA_REF_CHKP reference.
> > * cgraph.c (cgraph_remove_node): Fix instrumented_version
> > of the referenced node if any.
> > (dump_cgraph_node): Dump instrumentation_clone and
> > instrumented_version fields.
> > (verify_cgraph_node): Check correctness of IPA_REF_CHKP
> > references and instrumentation thunks.
> > * cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
> > reference.
> > (cgraph_rebuild_references): Likewise.
> > * cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
> > calling instrumneted function version.
> > * ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
> > (ipa_ref): increase size of use field.
> > * ipa-ref.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
> > * lto-cgraph.c (lto_output_node): Output instrumentation_clone,
> > thunk.add_pointer_bounds_args and orig_decl field.
> > (lto_output_ref): Adjust to new ipa_ref::use field size.
> > (input_overwrite_node): Read instrumentation_clone field.
> > (input_node): Read thunk.add_pointer_bounds_args and orig_decl
> > fields.
> > (input_ref): Adjust to new ipa_ref::use field size.
> > (input_cgraph_1): Compute instrumented_version fields and restore
> > IDENTIFIER_TRANSPARENT_ALIAS chains.
> > * lto-streamer.h (LTO_minor_version): Change minor version from
> > 0 to 1.
> > * ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
> > clone as address taken if the original one is address taken.
> > (cgraph_externally_visible_p): Mark instrumented 'main' as
> > externally visible.
> > (function_and_variable_visibility): Filter instrumentation
> > thunks.
> Thanks for adding the additional checking.
>
>
> >@@ -513,6 +517,11 @@ cgraph_rebuild_references (void)
> > ipa_record_stmt_references (node, gsi_stmt (gsi));
> > }
> > record_eh_tables (node, cfun);
> >+
> >+
> >+ if (node->instrumented_version
> >+ && !node->instrumentation_clone)
> >+ ipa_record_reference (node, node->instrumented_version, IPA_REF_CHKP, NULL);
> > }
> Trivial nit here -- just one vertical space here.
>
> With that nit fixed, this is OK.
>
> jeff
>
Thanks for review! Below is an updated version. It also includes usage of syntab classes. No functional changes added.
Thanks,
Ilya
--
2014-09-15 Ilya Enkovich <ilya.enkovich@intel.com>
* cgraph.h (cgraph_thunk_info): Add add_pointer_bounds_args
field.
(cgraph_node): Add instrumented_version, orig_decl and
instrumentation_clone fields.
(symtab_node::get_alias_target): Allow IPA_REF_CHKP reference.
* cgraph.c (cgraph_node::remove): Fix instrumented_version
of the referenced node if any.
(cgraph_node::dump): Dump instrumentation_clone and
instrumented_version fields.
(cgraph_node::verify_node): Check correctness of IPA_REF_CHKP
references and instrumentation thunks.
* cgraphbuild.c (rebuild_cgraph_edges): Rebuild IPA_REF_CHKP
reference.
(cgraph_rebuild_references): Likewise.
* cgraphunit.c (assemble_thunks_and_aliases): Skip thunks
calling instrumneted function version.
* ipa-ref.h (ipa_ref_use): Add IPA_REF_CHKP.
(ipa_ref): increase size of use field.
* symtab.c (ipa_ref_use_name): Add element for IPA_REF_CHKP.
* lto-cgraph.c (lto_output_node): Output instrumentation_clone,
thunk.add_pointer_bounds_args and orig_decl field.
(lto_output_ref): Adjust to new ipa_ref::use field size.
(input_overwrite_node): Read instrumentation_clone field.
(input_node): Read thunk.add_pointer_bounds_args and orig_decl
fields.
(input_ref): Adjust to new ipa_ref::use field size.
(input_cgraph_1): Compute instrumented_version fields and restore
IDENTIFIER_TRANSPARENT_ALIAS chains.
* lto-streamer.h (LTO_minor_version): Change minor version from
0 to 1.
* ipa.c (symtab_remove_unreachable_nodes): Consider instrumented
clone as address taken if the original one is address taken.
* ipa-visibility.c (cgraph_externally_visible_p): Mark
instrumented 'main' as externally visible.
(function_and_variable_visibility): Filter instrumentation
thunks.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 5a0b903..d8fdda1 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1823,6 +1823,12 @@ cgraph_node::remove (void)
}
cgraph_n_nodes--;
+ if (instrumented_version)
+ {
+ instrumented_version->instrumented_version = NULL;
+ instrumented_version = NULL;
+ }
+
/* Clear out the node to NULL all pointers and add the node to the free
list. */
memset (this, 0, sizeof (*this));
@@ -2069,6 +2075,11 @@ cgraph_node::dump (FILE *f)
if (indirect_calls_count)
fprintf (f, " Has %i outgoing edges for indirect calls.\n",
indirect_calls_count);
+
+ if (instrumentation_clone)
+ fprintf (f, " Is instrumented version.\n");
+ else if (instrumented_version)
+ fprintf (f, " Has instrumented version.\n");
}
/* Dump call graph node NODE to stderr. */
@@ -2834,7 +2845,9 @@ cgraph_node::verify_node (void)
error_found = true;
}
for (i = 0; iterate_reference (i, ref); i++)
- if (ref->use != IPA_REF_ALIAS)
+ if (ref->use == IPA_REF_CHKP)
+ ;
+ else if (ref->use != IPA_REF_ALIAS)
{
error ("Alias has non-alias reference");
error_found = true;
@@ -2852,6 +2865,64 @@ cgraph_node::verify_node (void)
error_found = true;
}
}
+
+ /* Check instrumented version reference. */
+ if (instrumented_version
+ && instrumented_version->instrumented_version != this)
+ {
+ error ("Instrumentation clone does not reference original node");
+ error_found = true;
+ }
+
+ /* Cannot have orig_decl for not instrumented nodes. */
+ if (!instrumentation_clone && orig_decl)
+ {
+ error ("Not instrumented node has non-NULL original declaration");
+ error_found = true;
+ }
+
+ /* If original not instrumented node still exists then we may check
+ original declaration is set properly. */
+ if (instrumented_version
+ && orig_decl
+ && orig_decl != instrumented_version->decl)
+ {
+ error ("Instrumented node has wrong original declaration");
+ error_found = true;
+ }
+
+ /* Check all nodes have chkp reference to their instrumented versions. */
+ if (analyzed
+ && instrumented_version
+ && !instrumentation_clone)
+ {
+ bool ref_found = false;
+ int i;
+ struct ipa_ref *ref;
+
+ for (i = 0; iterate_reference (i, ref); i++)
+ if (ref->use == IPA_REF_CHKP)
+ {
+ if (ref_found)
+ {
+ error ("Node has more than one chkp reference");
+ error_found = true;
+ }
+ if (ref->referred != instrumented_version)
+ {
+ error ("Wrong node is referenced with chkp reference");
+ error_found = true;
+ }
+ ref_found = true;
+ }
+
+ if (!ref_found)
+ {
+ error ("Analyzed node has no reference to instrumented version");
+ error_found = true;
+ }
+ }
+
if (analyzed && thunk.thunk_p)
{
if (!callees)
@@ -2869,6 +2940,12 @@ cgraph_node::verify_node (void)
error ("Thunk is not supposed to have body");
error_found = true;
}
+ if (thunk.add_pointer_bounds_args
+ && !instrumented_version->semantically_equivalent_p (callees->callee))
+ {
+ error ("Instrumentation thunk has wrong edge callee");
+ error_found = true;
+ }
}
else if (analyzed && gimple_has_body_p (decl)
&& !TREE_ASM_WRITTEN (decl)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index eb1aad6..cc045f6 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -516,6 +516,7 @@ struct GTY(()) cgraph_thunk_info {
tree alias;
bool this_adjusting;
bool virtual_offset_p;
+ bool add_pointer_bounds_args;
/* Set to true when alias node is thunk. */
bool thunk_p;
};
@@ -1139,6 +1140,13 @@ public:
cgraph_node *prev_sibling_clone;
cgraph_node *clones;
cgraph_node *clone_of;
+ /* If instrumentation_clone is 1 then instrumented_version points
+ to the original function used to make instrumented version.
+ Otherwise points to instrumented version of the function. */
+ cgraph_node *instrumented_version;
+ /* If instrumentation_clone is 1 then orig_decl is the original
+ function declaration. */
+ tree orig_decl;
/* For functions with many calls sites it holds map from call expression
to the edge to speed up cgraph_edge function. */
htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
@@ -1199,6 +1207,9 @@ public:
/* True if this decl calls a COMDAT-local function. This is set up in
compute_inline_parameters and inline_call. */
unsigned calls_comdat_local : 1;
+ /* True when function is clone created for Pointer Bounds Checker
+ instrumentation. */
+ unsigned instrumentation_clone : 1;
};
/* A cgraph node set is a collection of cgraph nodes. A cgraph node
@@ -1690,6 +1701,8 @@ symtab_node::get_alias_target (void)
{
struct ipa_ref *ref = NULL;
iterate_reference (0, ref);
+ if (ref->use == IPA_REF_CHKP)
+ iterate_reference (1, ref);
gcc_checking_assert (ref->use == IPA_REF_ALIAS);
return ref->referred;
}
diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index a04958f..f3e6f17 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -458,6 +458,10 @@ rebuild_cgraph_edges (void)
record_eh_tables (node, cfun);
gcc_assert (!node->global.inlined_to);
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ node->add_reference (node->instrumented_version, IPA_REF_CHKP, NULL);
+
return 0;
}
@@ -490,6 +494,10 @@ cgraph_rebuild_references (void)
node->record_stmt_references (gsi_stmt (gsi));
}
record_eh_tables (node, cfun);
+
+ if (node->instrumented_version
+ && !node->instrumentation_clone)
+ node->add_reference (node->instrumented_version, IPA_REF_CHKP, NULL);
}
namespace {
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index d9acc65..52e4242 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1705,7 +1705,8 @@ assemble_thunks_and_aliases (struct cgraph_node *node)
struct ipa_ref *ref;
for (e = node->callers; e;)
- if (e->caller->thunk.thunk_p)
+ if (e->caller->thunk.thunk_p
+ && !e->caller->thunk.add_pointer_bounds_args)
{
struct cgraph_node *thunk = e->caller;
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index b8b1f9e..e0d8ffd 100644
--- a/gcc/ipa-ref.h
+++ b/gcc/ipa-ref.h
@@ -29,7 +29,8 @@ enum GTY(()) ipa_ref_use
IPA_REF_LOAD,
IPA_REF_STORE,
IPA_REF_ADDR,
- IPA_REF_ALIAS
+ IPA_REF_ALIAS,
+ IPA_REF_CHKP
};
/* Record of reference in callgraph or varpool. */
@@ -54,7 +55,7 @@ public:
gimple stmt;
unsigned int lto_stmt_uid;
unsigned int referred_index;
- ENUM_BITFIELD (ipa_ref_use) use:2;
+ ENUM_BITFIELD (ipa_ref_use) use:3;
unsigned int speculative:1;
};
diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
index bca2bc7..4fdf804 100644
--- a/gcc/ipa-visibility.c
+++ b/gcc/ipa-visibility.c
@@ -260,6 +260,10 @@ cgraph_externally_visible_p (struct cgraph_node *node,
if (MAIN_NAME_P (DECL_NAME (node->decl)))
return true;
+ if (node->instrumentation_clone
+ && MAIN_NAME_P (DECL_NAME (node->orig_decl)))
+ return true;
+
return false;
}
@@ -535,6 +539,7 @@ function_and_variable_visibility (bool whole_program)
}
if (node->thunk.thunk_p
+ && !node->thunk.add_pointer_bounds_args
&& TREE_PUBLIC (node->decl))
{
struct cgraph_node *decl_node = node;
diff --git a/gcc/ipa.c b/gcc/ipa.c
index ce0fc34..a37ad03 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -490,6 +490,12 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
node->remove_from_same_comdat_group ();
node->remove_all_references ();
changed = true;
+ if (node->thunk.thunk_p
+ && node->thunk.add_pointer_bounds_args)
+ {
+ node->thunk.thunk_p = false;
+ node->thunk.add_pointer_bounds_args = false;
+ }
}
}
else
@@ -564,7 +570,10 @@ symtab_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
&& !node->used_from_other_partition)
{
if (!node->call_for_symbol_thunks_and_aliases
- (has_addr_references_p, NULL, true))
+ (has_addr_references_p, NULL, true)
+ && (!node->instrumentation_clone
+ || !node->instrumented_version
+ || !node->instrumented_version->address_taken))
{
if (file)
fprintf (file, " %s", node->name ());
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index ed715f1..3cbff8a 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -541,6 +541,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
bp_pack_value (&bp, node->thunk.thunk_p && !boundary_p, 1);
bp_pack_enum (&bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN, node->resolution);
+ bp_pack_value (&bp, node->instrumentation_clone, 1);
streamer_write_bitpack (&bp);
streamer_write_data_stream (ob->main_stream, section, strlen (section) + 1);
@@ -549,7 +550,8 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
streamer_write_uhwi_stream
(ob->main_stream,
1 + (node->thunk.this_adjusting != 0) * 2
- + (node->thunk.virtual_offset_p != 0) * 4);
+ + (node->thunk.virtual_offset_p != 0) * 4
+ + (node->thunk.add_pointer_bounds_args != 0) * 8);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.fixed_offset);
streamer_write_uhwi_stream (ob->main_stream, node->thunk.virtual_value);
}
@@ -558,6 +560,9 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
streamer_write_hwi_stream (ob->main_stream, node->get_init_priority ());
if (DECL_STATIC_DESTRUCTOR (node->decl))
streamer_write_hwi_stream (ob->main_stream, node->get_fini_priority ());
+
+ if (node->instrumentation_clone)
+ lto_output_fn_decl_index (ob->decl_state, ob->main_stream, node->orig_decl);
}
/* Output the varpool NODE to OB.
@@ -656,7 +661,7 @@ lto_output_ref (struct lto_simple_output_block *ob, struct ipa_ref *ref,
struct cgraph_node *node;
bp = bitpack_create (ob->main_stream);
- bp_pack_value (&bp, ref->use, 2);
+ bp_pack_value (&bp, ref->use, 3);
bp_pack_value (&bp, ref->speculative, 1);
streamer_write_bitpack (&bp);
nref = lto_symtab_encoder_lookup (encoder, ref->referred);
@@ -1080,6 +1085,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
node->thunk.thunk_p = bp_unpack_value (bp, 1);
node->resolution = bp_unpack_enum (bp, ld_plugin_symbol_resolution,
LDPR_NUM_KNOWN);
+ node->instrumentation_clone = bp_unpack_value (bp, 1);
gcc_assert (flag_ltrans
|| (!node->in_other_partition
&& !node->used_from_other_partition));
@@ -1203,6 +1209,7 @@ input_node (struct lto_file_decl_data *file_data,
node->thunk.this_adjusting = (type & 2);
node->thunk.virtual_value = virtual_value;
node->thunk.virtual_offset_p = (type & 4);
+ node->thunk.add_pointer_bounds_args = (type & 8);
}
if (node->alias && !node->analyzed && node->weakref)
node->alias_target = get_alias_symbol (node->decl);
@@ -1211,6 +1218,14 @@ input_node (struct lto_file_decl_data *file_data,
node->set_init_priority (streamer_read_hwi (ib));
if (DECL_STATIC_DESTRUCTOR (node->decl))
node->set_fini_priority (streamer_read_hwi (ib));
+
+ if (node->instrumentation_clone)
+ {
+ decl_index = streamer_read_uhwi (ib);
+ fn_decl = lto_file_decl_data_get_fn_decl (file_data, decl_index);
+ node->orig_decl = fn_decl;
+ }
+
return node;
}
@@ -1307,7 +1322,7 @@ input_ref (struct lto_input_block *ib,
struct ipa_ref *ref;
bp = streamer_read_bitpack (ib);
- use = (enum ipa_ref_use) bp_unpack_value (&bp, 2);
+ use = (enum ipa_ref_use) bp_unpack_value (&bp, 3);
speculative = (enum ipa_ref_use) bp_unpack_value (&bp, 1);
node = nodes[streamer_read_hwi (ib)];
ref = referring_node->add_reference (node, use);
@@ -1449,6 +1464,22 @@ input_cgraph_1 (struct lto_file_decl_data *file_data,
= dyn_cast<cgraph_node *> (nodes[ref]);
else
cnode->global.inlined_to = NULL;
+
+ /* Compute instrumented_version. */
+ if (cnode->instrumentation_clone)
+ {
+ gcc_assert (cnode->orig_decl);
+
+ cnode->instrumented_version = cgraph_node::get (cnode->orig_decl);
+ if (cnode->instrumented_version)
+ cnode->instrumented_version->instrumented_version = cnode;
+
+ /* Restore decl names reference. */
+ if (IDENTIFIER_TRANSPARENT_ALIAS (DECL_ASSEMBLER_NAME (cnode->decl))
+ && !TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl)))
+ TREE_CHAIN (DECL_ASSEMBLER_NAME (cnode->decl))
+ = DECL_ASSEMBLER_NAME (cnode->orig_decl);
+ }
}
ref = (int) (intptr_t) node->same_comdat_group;
diff --git a/gcc/symtab.c b/gcc/symtab.c
index a93c299..f975682 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -45,7 +45,7 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-utils.h"
#include "calls.h"
-static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
+static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
const char * const ld_plugin_symbol_resolution_names[]=
{
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-15 7:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-16 14:16 [PATCH, Pointer Bounds Checker 9/x] Cgraph extension Ilya Enkovich
2014-05-06 12:14 ` Ilya Enkovich
2014-06-27 8:12 ` Ilya Enkovich
2014-07-23 13:47 ` Jeff Law
2014-07-24 9:59 ` Ilya Enkovich
2014-07-24 12:05 ` Jan Hubicka
2014-07-24 13:19 ` Ilya Enkovich
2014-07-24 13:52 ` Jan Hubicka
2014-07-25 11:20 ` Ilya Enkovich
2014-09-03 19:32 ` Jeff Law
2014-09-15 7:51 ` Ilya Enkovich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).