On 11/14/14 01:22, Ilya Enkovich wrote:
2014-11-14 9:43 GMT+03:00 Jeff Law <l...@redhat.com>:
On 11/06/14 04:48, Ilya Enkovich wrote:
--
2014-11-06 Ilya Enkovich <ilya.enkov...@intel.com>
* tree-core.h (built_in_class): Add builtin codes to be used
by Pointer Bounds Checker for instrumented builtin functions.
* tree-streamer-in.c: Include ipa-chkp.h.
(streamer_get_builtin_tree): Create instrumented decl if
required.
* ipa-chkp.h (chkp_maybe_clone_builtin_fndecl): New.
* ipa-chkp.c (chkp_build_instrumented_fndecl): Support builtin
function decls.
(chkp_maybe_clone_builtin_fndecl): New.
(chkp_maybe_create_clone): Support builtin function decls.
Looks much better than prior versions.
@@ -355,6 +365,30 @@ chkp_add_bounds_params_to_function (tree fndecl)
chkp_copy_function_type_adding_bounds (TREE_TYPE (fndecl));
}
+tree
+chkp_maybe_clone_builtin_fndecl (tree fndecl)
Need a function comment here.
/* Return clone created for instrumentation of NODE or NULL. */
cgraph_node *
@@ -365,6 +399,52 @@ chkp_maybe_create_clone (tree fndecl)
gcc_assert (!node->instrumentation_clone);
+ if (DECL_BUILT_IN (fndecl)
+ && (DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL
+ || DECL_FUNCTION_CODE (fndecl) >= BEGIN_CHKP_BUILTINS))
+ return NULL;
Just so I'm sure, the only way to get into chkp_maybe_clone_builtin_decl is
if this test is false. Right?
Can we ultimately end up with checking clones for any normal builtin? What
filters out builtins that don't need a checking variant?
As you already noticed the filter is in the second patch.
+
+ clone = node->instrumented_version;
+
+ /* For builtin functions we may loose and recreate
+ cgraph node. We should check if we already have
+ instrumented version. */
Can you describe to me under what circumstances this happens? It seems like
we may be papering over an issue that would be better fixed elsewhere.
I don't think I'm hiding some problem here. Builtin function calls
may be removed during various optimizations. Therefore we may remove
all calls to some instrumented builtins and corresponding cgraph_node
is removed as unreachable (but fndecl still exists). Later calls to
removed function may be created again. IIRC in my test case it
happened in strlen pass which may replace builtin calls with another
ones. In this case cgraph_node is recreated but fndecl recreation
should be avoided, existing one should be used instead.
OK. Please add those details to the comment. With that comment updated,
OK for the trunk.
jeff
