Matthew Malcomson <matthew.malcom...@arm.com> writes:
> +/* Emit gimple statements into &stmts that take the size given in `len` and
> + generate a size that is guaranteed to be rounded upwards to `align`.
> +
> + This is a helper function for both handle_builtin_alloca and
> + asan_expand_mark_ifn when using HWASAN.
> +
> + Return the tree node representing this size, it is of TREE_TYPE
> + size_type_node. */
> +
> +static tree
> +hwasan_emit_round_up (gimple_seq *seq, location_t loc, tree old_size,
> + uint8_t align)
> +{
> + uint8_t tg_mask = align - 1;
> + /* tree new_size = (old_size + tg_mask) & ~tg_mask; */
> + tree tree_mask = build_int_cst (size_type_node, tg_mask);
> + tree oversize = gimple_build (seq, loc, PLUS_EXPR, size_type_node,
> old_size,
> + tree_mask);
> +
> + tree mask = build_int_cst (size_type_node, -align);
> + return gimple_build (seq, loc, BIT_AND_EXPR, size_type_node, oversize,
> mask);
> +}
> +
There's nothing really hwasan-specific about this, apart from the choice
“uint8_t” for the alignment and mask. So I think we should:
- chnage “align” and “tg_mask” to “unsigned HOST_WIDE_INT”
- change the name to “gimple_build_round_up”
- take the type as a parameter, in the same position as other
gimple_build_* type parameters
- move the function to gimple-fold.c, exported via gimple-fold.h
- drop:
This is a helper function for both handle_builtin_alloca and
asan_expand_mark_ifn when using HWASAN.
> […]
> @@ -690,6 +757,71 @@ handle_builtin_alloca (gcall *call, gimple_stmt_iterator
> *iter)
> = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA
> ? 0 : tree_to_uhwi (gimple_call_arg (call, 1));
>
> + if (hwasan_sanitize_allocas_p ())
> + {
> + gimple_seq stmts = NULL;
> + location_t loc = gimple_location (gsi_stmt (*iter));
> + /*
> + HWASAN needs a different expansion.
> +
> + addr = __builtin_alloca (size, align);
> +
> + should be replaced by
> +
> + new_size = size rounded up to HWASAN_TAG_GRANULE_SIZE byte alignment;
> + untagged_addr = __builtin_alloca (new_size, align);
> + tag = __hwasan_choose_alloca_tag ();
> + addr = ifn_HWASAN_SET_TAG (untagged_addr, tag);
> + __hwasan_tag_memory (untagged_addr, tag, new_size);
> + */
> + /* Ensure alignment at least HWASAN_TAG_GRANULE_SIZE bytes so we start
> on
> + a tag granule. */
> + align = align > HWASAN_TAG_GRANULE_SIZE ? align :
> HWASAN_TAG_GRANULE_SIZE;
> +
> + tree old_size = gimple_call_arg (call, 0);
> + tree new_size = hwasan_emit_round_up (&stmts, loc, old_size,
> + HWASAN_TAG_GRANULE_SIZE);
> +
> + /* Make the alloca call */
> + tree untagged_addr
> + = gimple_build (&stmts, loc,
> + as_combined_fn (BUILT_IN_ALLOCA_WITH_ALIGN), ptr_type,
> + new_size, build_int_cst (size_type_node, align));
> +
> + /* Choose the tag.
> + Here we use an internal function so we can choose the tag at expand
> + time. We need the decision to be made after stack variables have been
> + assigned their tag (i.e. once the hwasan_frame_tag_offset variable has
> + been set to one after the last stack variables tag). */
> + gcall *stmt = gimple_build_call_internal (IFN_HWASAN_CHOOSE_TAG, 0);
> + tree tag = make_ssa_name (unsigned_char_type_node);
> + gimple_call_set_lhs (stmt, tag);
> + gimple_set_location (stmt, loc);
> + gimple_seq_add_stmt_without_update (&stmts, stmt);
Even though there are currently no folds defined for argumentless
functions, I think it would be worth adding a gimple_build overload
for this instead of writing it out by hand. I.e. have a zero-argument
version of:
tree
gimple_build (gimple_seq *seq, location_t loc, combined_fn fn,
tree type, tree arg0)
{
tree res = gimple_simplify (fn, type, arg0, seq, gimple_build_valueize);
if (!res)
{
gcall *stmt;
if (internal_fn_p (fn))
stmt = gimple_build_call_internal (as_internal_fn (fn), 1, arg0);
else
{
tree decl = builtin_decl_implicit (as_builtin_fn (fn));
stmt = gimple_build_call (decl, 1, arg0);
}
if (!VOID_TYPE_P (type))
{
res = create_tmp_reg_or_ssa_name (type);
gimple_call_set_lhs (stmt, res);
}
gimple_set_location (stmt, loc);
gimple_seq_add_stmt_without_update (seq, stmt);
}
return res;
}
without the gimple_simplify call.
> +
> + /* Add tag to pointer. */
> + tree addr
> + = gimple_build (&stmts, loc, as_combined_fn (IFN_HWASAN_SET_TAG),
This is CFN_HWASAN_SET_TAG.
> + ptr_type, untagged_addr, tag);
> +
> + /* Tag shadow memory.
> + NOTE: require using `untagged_addr` here for libhwasan API. */
> + gimple_build (&stmts, loc, as_combined_fn (BUILT_IN_HWASAN_TAG_MEM),
> + void_type_node, untagged_addr, tag, new_size);
> +
> + /* Insert the built up code sequence into the original instruction
> stream
> + the iterator points to. */
> + gsi_insert_seq_before (iter, stmts, GSI_SAME_STMT);
> +
> + /* Finally, replace old alloca ptr with NEW_ALLOCA. */
> + replace_call_with_value (iter, addr);
> + return;
> + }
> +
> + tree last_alloca = get_last_alloca_addr ();
> + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1;
> +
> +
Nit: excess blank line.
> /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN
> bytes of allocated space. Otherwise, align alloca to ASAN_RED_ZONE_SIZE
> manually. */
> […]
> @@ -2351,7 +2686,8 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
> {
> if (DECL_THREAD_LOCAL_P (inner))
> return;
> - if (!param_asan_globals && is_global_var (inner))
> + if ((hwasan_sanitize_p () || !param_asan_globals)
> + && is_global_var (inner))
> return;
> if (!TREE_STATIC (inner))
> {
As mentioned in the previous round, I think it would be good to have
some commentary explaining this.
> […]
> + /* If the length is a simple SHWI, then we can calculate the rounded up
> + length directly. Otherwise we need to emit gimple to do this
> + rounding at runtime. */
> + if (tree_fits_shwi_p (len))
> + {
> + unsigned HOST_WIDE_INT size_in_bytes = tree_to_shwi (len);
> + uint8_t tg_mask = HWASAN_TAG_GRANULE_SIZE - 1;
> + size_in_bytes = (size_in_bytes + tg_mask) & ~tg_mask;
> + new_len = build_int_cst (pointer_sized_int_node, size_in_bytes);
> + }
> + else
> + new_len = hwasan_emit_round_up (&stmts, loc, len,
> + HWASAN_TAG_GRANULE_SIZE);
I'm not sure the special case is worth it. We might as well call
(hwasan_emit/gimple_build)_round_up unconditionally, which would also
have the nice effect of testing it more.
> + gimple_build (&stmts, loc, as_combined_fn (IFN_HWASAN_MARK),
CFN_HWASAN_MARK
> + void_type_node, gimple_call_arg (g, 0),
> + base, new_len);
> + gsi_replace_with_seq (iter, stmts, true);
> + return false;
> + }
> +
> if (is_poison)
> {
> if (asan_handled_variables == NULL)
> […]
> +/* For hwasan stack tagging:
> + Tag a region of space in the shadow stack according to the base pointer of
> + an object on the stack. N.b. the length provided in the internal call is
> + required to be aligned to HWASAN_TAG_GRANULE_SIZE. */
> +static void
> +expand_HWASAN_MARK (internal_fn, gcall *gc)
> +{
> + HOST_WIDE_INT flag = tree_to_shwi (gimple_call_arg (gc, 0));
> + bool is_poison = ((asan_mark_flags)flag) == ASAN_MARK_POISON;
> +
> + tree base = gimple_call_arg (gc, 1);
> + gcc_checking_assert (TREE_CODE (base) == ADDR_EXPR);
> + /* base is a pointer argument, hence in ptr_mode.
> + We convert to Pmode for use in the targetm.memtag.extract_tag and
> + targetm.memtag.untagged_pointer hooks.
> + We then convert the result to ptr_mode for the emit_library_call.
> +
> + This conversion is not for functionality so much as for the code to be
> + correctly formed. If ptr_mode is smaller than Pmode then the top byte
> of
> + a Pmode value will be truncated in C code losing the tag values, and
> + HWASAN will not work. */
> + rtx base_rtx = convert_memory_address (Pmode, expand_normal (base));
> +
> + rtx tag = is_poison ? HWASAN_STACK_BACKGROUND
> + : targetm.memtag.extract_tag (base_rtx, NULL_RTX);
> + rtx address = targetm.memtag.untagged_pointer (base_rtx, NULL_RTX);
> + address = convert_memory_address (ptr_mode, address);
> +
> + tree len = gimple_call_arg (gc, 2);
> + rtx r_len = expand_normal (len);
> +
> + rtx func = init_one_libfunc ("__hwasan_tag_memory");
> + emit_library_call (func, LCT_NORMAL, VOIDmode, address, ptr_mode,
> + tag, QImode, r_len, ptr_mode);
> +}
> +
> +/* For hwasan stack tagging:
> + Store a tag into a pointer. */
> +static void
> +expand_HWASAN_SET_TAG (internal_fn, gcall *gc)
> +{
> + tree g_target = gimple_call_lhs (gc);
> + tree g_ptr = gimple_call_arg (gc, 0);
> + tree g_tag = gimple_call_arg (gc, 1);
> +
> + rtx ptr = convert_memory_address (Pmode, expand_normal (g_ptr));
> + rtx tag = expand_expr (g_tag, NULL_RTX, QImode, EXPAND_NORMAL);
> + rtx target = expand_normal (g_target);
> + machine_mode mode = GET_MODE (target);
> +
> + rtx untagged = targetm.memtag.untagged_pointer (ptr, target);
> + rtx tagged_value = targetm.memtag.set_tag (untagged, tag, target);
> + /* Really need to put the return value into the `target` RTX, since that's
> + the return value of the function.
> + `target` will be in ptr_mode, while `tagged_value` will be in Pmode.
> + These can be different. When they are different we try to truncate the
> + Pmode value into ptr_mode. This will usually lose the tag, but since
> such
> + a difference between ptr_mode and Pmode will already cause problems
> + wherever the HWASAN library returns a pointer losing the tag here does
> not
> + introduce new incompatibilities.
> +
> + We do this mainly so that compiling for such a target with ptr_mode and
> + Pmode sizes being different doesn't ICE, even if the resulting binary is
> + not usable. */
I'm not sure that's the right trade-off. Producing wrong code is a more
serious class of bug than an ICE.
Could the code in principle be reached on ILP32 targets? If so,
that seems like something we should fix. If not, then I think we
should just do everything in Pmode, perhaps with an assertion that
Pmode == ptr_mode to make it clear that this is a deliberate choice.
I see what you mean about trying to make sure that types “agree”
at the appropriate interface. But in a sense we're already failing
to do that because we assume Pmode is correct for all addresses,
without taking address spaces into account. Assuming Pmode == ptr_mode
doesn't seem worse than assuming a unified address space.
LGTM otherwise though.
Thanks,
Richard
