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?
+
+ 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.
@@ -409,6 +489,15 @@ chkp_maybe_create_clone (tree fndecl)
actually copies args list from the original decl. */
chkp_add_bounds_params_to_function (new_decl);
+ /* Remember builtin fndecl. */
+ if (DECL_BUILT_IN_CLASS (clone->decl) == BUILT_IN_NORMAL
+ && fndecl == builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+ {
+ gcc_assert (!builtin_decl_explicit (DECL_FUNCTION_CODE
(clone->decl)));
+ set_builtin_decl (DECL_FUNCTION_CODE (clone->decl),
+ clone->decl, false);
+ }
I'm not a big fan of slamming in a new DECL like this, but it may be OK.
I'm not going to object to that now, but I worry about downstream impacts.
Tentatively OK after adding the missing function comment. Please wait
for entire kit to be approved before committing anything. I may come
back to something as I dig deeper into the other patches in the series.
jeff
