On 06/05/14 08:46, Ilya Enkovich wrote:
2014-06-05 Ilya Enkovich <ilya.enkov...@intel.com>
* calls.c: Include tree-chkp.h, rtl-chkp.h, bitmap.h.
(arg_data): Add fields special_slot, pointer_arg and
pointer_offset.
(store_bounds): New.
(emit_call_1): Propagate instrumentation flag for CALL.
(initialize_argument_information): Compute pointer_arg,
pointer_offset and special_slot for pointer bounds arguments.
(finalize_must_preallocate): Preallocate when storing bounds
in bounds table.
(compute_argument_addresses): Skip pointer bounds.
(expand_call): Store bounds into tables separately. Return
result joined with resulting bounds.
* cfgexpand.c: Include tree-chkp.h, rtl-chkp.h.
(expand_call_stmt): Propagate bounds flag for CALL_EXPR.
(expand_return): Add returned bounds arg. Handle returned bounds.
(expand_gimple_stmt_1): Adjust to new expand_return signature.
(gimple_expand_cfg): Reset rtx bounds map.
* expr.c: Include tree-chkp.h, rtl-chkp.h.
(expand_assignment): Handle returned bounds.
(store_expr_with_bounds): New. Replaces store_expr with new bounds
target argument. Handle bounds returned by calls.
(store_expr): Now wraps store_expr_with_bounds.
* expr.h (store_expr_with_bounds): New.
* function.c: Include tree-chkp.h, rtl-chkp.h.
(bounds_parm_data): New.
(use_register_for_decl): Do not registerize decls used for bounds
stores and loads.
(assign_parms_augmented_arg_list): Add bounds of the result
structure pointer as the second argument.
(assign_parm_find_entry_rtl): Mark bounds are never passed on
the stack.
(assign_parm_is_stack_parm): Likewise.
(assign_parm_load_bounds): New.
(assign_bounds): New.
(assign_parms): Load bounds and determine a location for
returned bounds.
(diddle_return_value_1): New.
(diddle_return_value): Handle returned bounds.
* function.h (rtl_data): Add field for returned bounds.
diff --git a/gcc/calls.c b/gcc/calls.c
index e1dc8eb..5fbbe9f 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -44,11 +44,14 @@ along with GCC; see the file COPYING3. If not see
#include "tm_p.h"
#include "timevar.h"
#include "sbitmap.h"
+#include "bitmap.h"
#include "langhooks.h"
#include "target.h"
#include "cgraph.h"
#include "except.h"
#include "dbgcnt.h"
+#include "tree-chkp.h"
+#include "rtl-chkp.h"
/* Like PREFERRED_STACK_BOUNDARY but in units of bytes, not bits. */
#define STACK_BYTES (PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
@@ -76,6 +79,15 @@ struct arg_data
/* If REG is a PARALLEL, this is a copy of VALUE pulled into the correct
form for emit_group_move. */
rtx parallel_value;
+ /* If value is passed in neither reg nor stack, this field holds a number
+ of a special slot to be used. */
+ rtx special_slot;
I really dislike "special_slot" and the comment here. The comment that
it's neither a reg nor stack is just bogus. What hardware resource does
"special_slot" refer to? It's a register, but one that we do not
typically expose. Let's at least clarify the comment and then we'll see
if something other than "special_slot" as a name makes sense. Yes, I
realize this is a bit of bikeshedding, but when the comments/terminology
is confusing, the code becomes even harder to understand.
I'm a bit concerned that this is exposing more details of the MPX
implementation than is advisable to the front/middle end. On the other
hand, I'd expect any other implementation that seeks to work in a
transparent manner is going to have many of the same implementation
properties as we see with MPX, so perhaps it's not a major problem.
@@ -1141,18 +1158,84 @@ initialize_argument_information (int num_actuals
ATTRIBUTE_UNUSED,
/* First fill in the actual arguments in the ARGS array, splitting
complex arguments if necessary. */
{
- int j = i;
+ int j = i, ptr_arg = -1;
call_expr_arg_iterator iter;
tree arg;
+ bitmap slots = NULL;
if (struct_value_addr_value)
{
args[j].tree_value = struct_value_addr_value;
+
j += inc;
+
+ /* If we pass structure address then we need to
+ create bounds for it. Since created bounds is
+ a call statement, we expand it right here to avoid
+ fixing all other places where it may be expanded. */
+ if (CALL_WITH_BOUNDS_P (exp))
+ {
+ args[j].value = gen_reg_rtx (targetm.chkp_bound_mode ());
+ args[j].tree_value
+ = chkp_make_bounds_for_struct_addr (struct_value_addr_value);
+ expand_expr_real (args[j].tree_value, args[j].value, VOIDmode,
+ EXPAND_NORMAL, 0, false);
+ args[j].pointer_arg = j - inc;
+
+ j += inc;
+ }
Just an FYI, I'm pretty sure this hunk isn't going to apply cleanly as
the context has changed on the trunk. I'd recommend getting this code
updated for the trunk. I suspect you're getting close to having all the
basic functionality bits in, you're obviously going to need to do a new
bootstrap & regression test prior to checkin. I think git sqashing the
series and testing/committing them as an atomic unit is probably wise.
It's been a while since I looked at this code, but is it safe to create
a new call tree at this point? I recall some major complications if you
try to insert a call once you've started filling in arguments. Hmm,
gIven you're at the start of initialize_argument_information, you're
probably OK since we haven't stored any arguments into their arg
regs/memory yet.
@@ -1302,6 +1388,12 @@ initialize_argument_information (int num_actuals
ATTRIBUTE_UNUSED,
args[i].reg = targetm.calls.function_arg (args_so_far, mode, type,
argpos < n_named_args);
+ if (args[i].reg && CONST_INT_P (args[i].reg))
+ {
+ args[i].special_slot = args[i].reg;
+ args[i].reg = NULL;
+ }
I can't recall from the earlier patches, but have you updated the
documentation to indicate that function_arg can return a CONST_INT?
I think this is mostly OK. If you could update and resend for another
once-over, it'd be appreciated.
Jeff
