On 14 Nov 23:58, Jeff Law wrote:
> On 11/14/14 01:06, Ilya Enkovich wrote:
>
> >>>- /* Avoid instrumented builtin functions for now. Due to IPA
> >>>- it also means we have to avoid instrumentation of indirect
> >>>- calls. */
> >>>- if (fndecl && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
> >>>- return;
> >>>+ /* We instrument only some subset of builtins. We also instrument
> >>>+ builtin calls to be inlined. */
> >>>+ if (fndecl
> >>>+ && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
> >>>+ && !chkp_instrument_normal_builtin (fndecl))
> >>>+ {
> >>>+ struct cgraph_node *clone = chkp_maybe_create_clone (fndecl);
> >>>+ if (!clone
> >>>+ || !gimple_has_body_p (clone->decl)
> >>>+ || !lookup_attribute ("always_inline", DECL_ATTRIBUTES
> >>>(fndecl)))
> >>>+ return;
> >>>+ }
> >>
> >>Is that outer conditional right? If we have a fndecl and it's a normal
> >>builtin, but it's _not_ one of hte builtins we're instrumenting, then call
> >>chkp_maybe_create_clone?
> >
> >Some builtin functions (especially their *_chk version) are defined as
> >always_inline functions in headers. In this case we handle them as
> >regular functions (clone and instrument) because they will be inlined
> >anyway. Seems gimple_has_body_p should be applied to fndecl and moved
> >into outer if-statement along with attribute check. Thus unneeded
> >clones would be avoided.
> So the outer condition is, essentially checking for a _chk builtin
> and if we find it, try to create a clone for instrumentation
> purposes.
>
> I think the always_inline attribute lookup can move to the outer-if.
> Less sure about the gimple_has_body check though. Presumably
> clone->decl is going to refer to fndecl? If so, then that can move
> to the outer if as well. As you say, that may cut down on the
> number of clones that get created.
>
> OK as-is, or with those two conditions moved out of level as long as
> all prerequisites are approved and it passes a bootstrap &
> regression test.
>
> JEff
I moved always_inline check up. Here is a final version.
Thanks,
Ilya
--
diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c
index 3e343d4..46b2139 100644
--- a/gcc/ipa-chkp.c
+++ b/gcc/ipa-chkp.c
@@ -581,8 +581,9 @@ chkp_versioning (void)
&& (!flag_chkp_instrument_marked_only
|| lookup_attribute ("bnd_instrument",
DECL_ATTRIBUTES (node->decl)))
- /* No builtins instrumentation for now. */
- && DECL_BUILT_IN_CLASS (node->decl) == NOT_BUILT_IN)
+ && (!DECL_BUILT_IN (node->decl)
+ || (DECL_BUILT_IN_CLASS (node->decl) == BUILT_IN_NORMAL
+ && DECL_FUNCTION_CODE (node->decl) < BEGIN_CHKP_BUILTINS)))
chkp_maybe_create_clone (node->decl);
}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index df7d425..0fb78cc 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -1586,6 +1586,50 @@ chkp_find_bound_slots (const_tree type, bitmap res)
chkp_find_bound_slots_1 (type, res, 0);
}
+/* Return 1 if call to FNDECL should be instrumented
+ and 0 otherwise. */
+
+static bool
+chkp_instrument_normal_builtin (tree fndecl)
+{
+ switch (DECL_FUNCTION_CODE (fndecl))
+ {
+ case BUILT_IN_STRLEN:
+ case BUILT_IN_STRCPY:
+ case BUILT_IN_STRNCPY:
+ case BUILT_IN_STPCPY:
+ case BUILT_IN_STPNCPY:
+ case BUILT_IN_STRCAT:
+ case BUILT_IN_STRNCAT:
+ case BUILT_IN_MEMCPY:
+ case BUILT_IN_MEMPCPY:
+ case BUILT_IN_MEMSET:
+ case BUILT_IN_MEMMOVE:
+ case BUILT_IN_BZERO:
+ case BUILT_IN_STRCMP:
+ case BUILT_IN_STRNCMP:
+ case BUILT_IN_BCMP:
+ case BUILT_IN_MEMCMP:
+ case BUILT_IN_MEMCPY_CHK:
+ case BUILT_IN_MEMPCPY_CHK:
+ case BUILT_IN_MEMMOVE_CHK:
+ case BUILT_IN_MEMSET_CHK:
+ case BUILT_IN_STRCPY_CHK:
+ case BUILT_IN_STRNCPY_CHK:
+ case BUILT_IN_STPCPY_CHK:
+ case BUILT_IN_STPNCPY_CHK:
+ case BUILT_IN_STRCAT_CHK:
+ case BUILT_IN_STRNCAT_CHK:
+ case BUILT_IN_MALLOC:
+ case BUILT_IN_CALLOC:
+ case BUILT_IN_REALLOC:
+ return 1;
+
+ default:
+ return 0;
+ }
+}
+
/* Add bound arguments to call statement pointed by GSI.
Also performs a replacement of user checker builtins calls
with internal ones. */
@@ -1619,7 +1663,7 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
&& DECL_FUNCTION_CODE (fndecl) == BUILT_IN_OBJECT_SIZE)
return;
- /* Donothing for calls to legacy functions. */
+ /* Do nothing for calls to legacy functions. */
if (fndecl
&& lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl)))
return;
@@ -1686,11 +1730,20 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
if (!flag_chkp_instrument_calls)
return;
- /* Avoid instrumented builtin functions for now. Due to IPA
- it also means we have to avoid instrumentation of indirect
- calls. */
- if (fndecl && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
- return;
+ /* We instrument only some subset of builtins. We also instrument
+ builtin calls to be inlined. */
+ if (fndecl
+ && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
+ && !chkp_instrument_normal_builtin (fndecl))
+ {
+ if (!lookup_attribute ("always_inline", DECL_ATTRIBUTES (fndecl)))
+ return;
+
+ struct cgraph_node *clone = chkp_maybe_create_clone (fndecl);
+ if (!clone
+ || !gimple_has_body_p (clone->decl))
+ return;
+ }
/* If function decl is available then use it for
formal arguments list. Otherwise use function type. */
@@ -1764,14 +1817,6 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi)
}
new_args.release ();
- /* If we call built-in function and pass no bounds then
- we do not need to change anything. */
- if (new_call == call
- && fndecl
- && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
- && fndecl == builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
- return;
-
/* For direct calls fndecl is replaced with instrumented version. */
if (fndecl)
{
@@ -3905,15 +3950,21 @@ chkp_replace_function_pointer (tree *op, int
*walk_subtrees,
{
if (TREE_CODE (*op) == FUNCTION_DECL
&& !lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (*op))
- /* Do not replace builtins for now. */
- && DECL_BUILT_IN_CLASS (*op) == NOT_BUILT_IN)
+ && (DECL_BUILT_IN_CLASS (*op) == NOT_BUILT_IN
+ /* For builtins we replace pointers only for selected
+ function and functions having definitions. */
+ || (DECL_BUILT_IN_CLASS (*op) == BUILT_IN_NORMAL
+ && (chkp_instrument_normal_builtin (*op)
+ || gimple_has_body_p (*op)))))
{
struct cgraph_node *node = cgraph_node::get_create (*op);
+ struct cgraph_node *clone = NULL;
if (!node->instrumentation_clone)
- chkp_maybe_create_clone (*op);
+ clone = chkp_maybe_create_clone (*op);
- *op = node->instrumented_version->decl;
+ if (clone)
+ *op = clone->decl;
*walk_subtrees = 0;
}
