libsanitizer merge from upstream r191666

On Tue, Oct 29, 2013 at 07:05:49AM -0700, Konstantin Serebryany wrote:
> The calls are emitted by default, but the __asan_stack_malloc call is
> done under a run-time flag
> __asan_option_detect_stack_use_after_return.
> So, to use the stack-use-after-return feature you should simply
> compile with -fsanitize=address and then at run-time
> pass ASAN_OPTIONS=detect_stack_use_after_return=1
> For small stack frame sizes the call to __asan_stack_free is inlined
> (as a performance optimization, not mandatory).

Ok, here is heavily untested implementation of the use after return
sanitization.  Tested just by eyeballing assembly generated for some
testcases and on stack-use-after-return.cc and testing asan.exp (but that
has no use-after-return tests, right?).

What I'm not very happy about is that __asan_stack_malloc_N doesn't have
align argument, is there at least some guaranteed alignment on what it
returns that gcc can hardcode (and if the required alignment is bigger, just
don't call __asan_stack_malloc_N)?  I mean, e.g. on x86_64/i686, right now
say for -mavx code quite often 256-bit alignment is needed, with -mavx512f
512-bit alignment will be needed from time to time.  So if for now
libasan could guarantee 64-byte alignment of the stack chunks or at least
32-byte alignment, I could pass the required alignment to
asan_emit_stack_protection and force no __asan_stack_malloc_N use if
the required alignment is smaller than the one guaranteed by libasan.

2013-11-14  Jakub Jelinek  <jakub@redhat.com>

	* cfgexpand.c (struct stack_vars_data): Add asan_base field.
	(expand_stack_vars): For -fsanitize=address, use (and set initially)
	data->asan_base as base for vars.
	(expand_used_vars): Initialize data.asan_base.  Pass it to
	asan_emit_stack_protection.
	* asan.c (asan_detect_stack_use_after_return): New variable.
	(asan_emit_stack_protection): Add pbase argument.  Implement use
	after return sanitization.
	* asan.h (asan_emit_stack_protection): Adjust prototype.
	(ASAN_STACK_MAGIC_USE_AFTER_RET, ASAN_STACK_RETIRED_MAGIC): Define.



	Jakub

On Thu, Nov 14, 2013 at 7:33 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Oct 29, 2013 at 07:05:49AM -0700, Konstantin Serebryany wrote:
>> The calls are emitted by default, but the __asan_stack_malloc call is
>> done under a run-time flag
>> __asan_option_detect_stack_use_after_return.
>> So, to use the stack-use-after-return feature you should simply
>> compile with -fsanitize=address and then at run-time
>> pass ASAN_OPTIONS=detect_stack_use_after_return=1
>> For small stack frame sizes the call to __asan_stack_free is inlined
>> (as a performance optimization, not mandatory).
>
> Ok, here is heavily untested implementation of the use after return
> sanitization.  Tested just by eyeballing assembly generated for some
> testcases and on stack-use-after-return.cc and testing asan.exp (but that
> has no use-after-return tests, right?).

Nice! I haven't tried gcc-asan on heavy things for a while.
I'll try to give it a shot (first w/o this patch, then with it) on
chrome, where stack-use-after-return is known to just work.


>
> What I'm not very happy about is that __asan_stack_malloc_N doesn't have
> align argument, is there at least some guaranteed alignment on what it
> returns that gcc

I thought about alignment but did not reflect it anywhere in the
interface/comments.
The alignment should be min(4096, N), which is enough for most purposes.
But let me double-check (and add a CHECK) tomorrow.

--kcc

> can hardcode (and if the required alignment is bigger, just
> don't call __asan_stack_malloc_N)?  I mean, e.g. on x86_64/i686, right now
> say for -mavx code quite often 256-bit alignment is needed, with -mavx512f
> 512-bit alignment will be needed from time to time.  So if for now
> libasan could guarantee 64-byte alignment of the stack chunks or at least
> 32-byte alignment, I could pass the required alignment to
> asan_emit_stack_protection and force no __asan_stack_malloc_N use if
> the required alignment is smaller than the one guaranteed by libasan.
>
> 2013-11-14  Jakub Jelinek  <jakub@redhat.com>
>
>         * cfgexpand.c (struct stack_vars_data): Add asan_base field.
>         (expand_stack_vars): For -fsanitize=address, use (and set initially)
>         data->asan_base as base for vars.
>         (expand_used_vars): Initialize data.asan_base.  Pass it to
>         asan_emit_stack_protection.
>         * asan.c (asan_detect_stack_use_after_return): New variable.
>         (asan_emit_stack_protection): Add pbase argument.  Implement use
>         after return sanitization.
>         * asan.h (asan_emit_stack_protection): Adjust prototype.
>         (ASAN_STACK_MAGIC_USE_AFTER_RET, ASAN_STACK_RETIRED_MAGIC): Define.
>
> --- gcc/cfgexpand.c.jj  2013-11-14 09:10:08.000000000 +0100
> +++ gcc/cfgexpand.c     2013-11-14 12:42:25.194538823 +0100
> @@ -879,6 +879,9 @@ struct stack_vars_data
>
>    /* Vector of partition representative decls in between the paddings.  */
>    vec<tree> asan_decl_vec;
> +
> +  /* Base pseudo register for Address Sanitizer protected automatic vars.  */
> +  rtx asan_base;
>  };
>
>  /* A subroutine of expand_used_vars.  Give each partition representative
> @@ -963,6 +966,7 @@ expand_stack_vars (bool (*pred) (size_t)
>        alignb = stack_vars[i].alignb;
>        if (alignb * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT)
>         {
> +         base = virtual_stack_vars_rtx;
>           if ((flag_sanitize & SANITIZE_ADDRESS) && pred)
>             {
>               HOST_WIDE_INT prev_offset = frame_offset;
> @@ -991,10 +995,12 @@ expand_stack_vars (bool (*pred) (size_t)
>               if (repr_decl == NULL_TREE)
>                 repr_decl = stack_vars[i].decl;
>               data->asan_decl_vec.safe_push (repr_decl);
> +             if (data->asan_base == NULL)
> +               data->asan_base = gen_reg_rtx (Pmode);
> +             base = data->asan_base;
>             }
>           else
>             offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
> -         base = virtual_stack_vars_rtx;
>           base_align = crtl->max_used_stack_slot_alignment;
>         }
>        else
> @@ -1768,6 +1774,7 @@ expand_used_vars (void)
>
>        data.asan_vec = vNULL;
>        data.asan_decl_vec = vNULL;
> +      data.asan_base = NULL_RTX;
>
>        /* Reorder decls to be protected by iterating over the variables
>          array multiple times, and allocating out of each phase in turn.  */
> @@ -1800,8 +1807,9 @@ expand_used_vars (void)
>
>           var_end_seq
>             = asan_emit_stack_protection (virtual_stack_vars_rtx,
> +                                         data.asan_base,
>                                           data.asan_vec.address (),
> -                                         data.asan_decl_vec. address (),
> +                                         data.asan_decl_vec.address (),
>                                           data.asan_vec.length ());
>         }
>
> --- gcc/asan.c.jj       2013-11-14 09:10:08.000000000 +0100
> +++ gcc/asan.c  2013-11-14 16:06:42.203713457 +0100
> @@ -226,6 +226,9 @@ alias_set_type asan_shadow_set = -1;
>     alias set is used for all shadow memory accesses.  */
>  static GTY(()) tree shadow_ptr_types[2];
>
> +/* Decl for __asan_option_detect_stack_use_after_return.  */
> +static GTY(()) tree asan_detect_stack_use_after_return;
> +
>  /* Hashtable support for memory references used by gimple
>     statements.  */
>
> @@ -939,20 +942,25 @@ asan_function_start (void)
>     and DECLS is an array of representative decls for each var partition.
>     LENGTH is the length of the OFFSETS array, DECLS array is LENGTH / 2 - 1
>     elements long (OFFSETS include gap before the first variable as well
> -   as gaps after each stack variable).  */
> +   as gaps after each stack variable).  PBASE is, if non-NULL, some pseudo
> +   register which stack vars DECL_RTLs are based on.  Either BASE should be
> +   assigned to PBASE, when not doing use after return protection, or
> +   corresponding address based on __asan_stack_malloc* return value.  */
>
>  rtx
> -asan_emit_stack_protection (rtx base, HOST_WIDE_INT *offsets, tree *decls,
> -                           int length)
> +asan_emit_stack_protection (rtx base, rtx pbase, HOST_WIDE_INT *offsets,
> +                           tree *decls, int length)
>  {
> -  rtx shadow_base, shadow_mem, ret, mem;
> +  rtx shadow_base, shadow_mem, ret, mem, orig_base, lab;
>    char buf[30];
>    unsigned char shadow_bytes[4];
>    HOST_WIDE_INT base_offset = offsets[length - 1], offset, prev_offset;
> +  HOST_WIDE_INT asan_frame_size = offsets[0] - base_offset;
>    HOST_WIDE_INT last_offset, last_size;
>    int l;
>    unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT;
>    tree str_cst, decl, id;
> +  int use_after_return_class = -1;
>
>    if (shadow_ptr_types[0] == NULL_TREE)
>      asan_init_shadow_ptr_types ();
> @@ -982,9 +990,53 @@ asan_emit_stack_protection (rtx base, HO
>    str_cst = asan_pp_string (&asan_pp);
>
>    /* Emit the prologue sequence.  */
> +  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase)
> +    use_after_return_class = floor_log2 (asan_frame_size - 1) - 5;
> +  if (use_after_return_class == -1 && pbase)
> +    emit_move_insn (pbase, base);
>    base = expand_binop (Pmode, add_optab, base,
>                        gen_int_mode (base_offset, Pmode),
>                        NULL_RTX, 1, OPTAB_DIRECT);
> +  orig_base = NULL_RTX;
> +  if (use_after_return_class != -1)
> +    {
> +      if (asan_detect_stack_use_after_return == NULL_TREE)
> +       {
> +         id = get_identifier ("__asan_option_detect_stack_use_after_return");
> +         decl = build_decl (BUILTINS_LOCATION, VAR_DECL, id,
> +                            integer_type_node);
> +         SET_DECL_ASSEMBLER_NAME (decl, id);
> +         TREE_ADDRESSABLE (decl) = 1;
> +         DECL_ARTIFICIAL (decl) = 1;
> +         DECL_IGNORED_P (decl) = 1;
> +         DECL_EXTERNAL (decl) = 1;
> +         TREE_STATIC (decl) = 1;
> +         TREE_PUBLIC (decl) = 1;
> +         TREE_USED (decl) = 1;
> +         asan_detect_stack_use_after_return = decl;
> +       }
> +      orig_base = gen_reg_rtx (Pmode);
> +      emit_move_insn (orig_base, base);
> +      ret = expand_normal (asan_detect_stack_use_after_return);
> +      lab = gen_label_rtx ();
> +      int very_likely = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / 2000 - 1);
> +      emit_cmp_and_jump_insns (ret, const0_rtx, EQ, NULL_RTX,
> +                              VOIDmode, 0, lab, very_likely);
> +      snprintf (buf, sizeof buf, "__asan_stack_malloc_%d",
> +               use_after_return_class);
> +      ret = init_one_libfunc (buf);
> +      rtx addr = convert_memory_address (ptr_mode, base);
> +      ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2,
> +                                    GEN_INT (asan_frame_size),
> +                                    TYPE_MODE (pointer_sized_int_node),
> +                                    addr, ptr_mode);
> +      ret = convert_memory_address (Pmode, ret);
> +      emit_move_insn (base, ret);
> +      emit_label (lab);
> +      emit_move_insn (pbase, expand_binop (Pmode, add_optab, base,
> +                                          gen_int_mode (-base_offset, Pmode),
> +                                          NULL_RTX, 1, OPTAB_DIRECT));
> +    }
>    mem = gen_rtx_MEM (ptr_mode, base);
>    emit_move_insn (mem, gen_int_mode (ASAN_STACK_FRAME_MAGIC, ptr_mode));
>    mem = adjust_address (mem, VOIDmode, GET_MODE_SIZE (ptr_mode));
> @@ -1006,10 +1058,8 @@ asan_emit_stack_protection (rtx base, HO
>    shadow_base = expand_binop (Pmode, lshr_optab, base,
>                               GEN_INT (ASAN_SHADOW_SHIFT),
>                               NULL_RTX, 1, OPTAB_DIRECT);
> -  shadow_base = expand_binop (Pmode, add_optab, shadow_base,
> -                             gen_int_mode (targetm.asan_shadow_offset (),
> -                                           Pmode),
> -                             NULL_RTX, 1, OPTAB_DIRECT);
> +  shadow_base
> +    = plus_constant (Pmode, shadow_base, targetm.asan_shadow_offset ());
>    gcc_assert (asan_shadow_set != -1
>               && (ASAN_RED_ZONE_SIZE >> ASAN_SHADOW_SHIFT) == 4);
>    shadow_mem = gen_rtx_MEM (SImode, shadow_base);
> @@ -1060,6 +1110,47 @@ asan_emit_stack_protection (rtx base, HO
>    /* Construct epilogue sequence.  */
>    start_sequence ();
>
> +  lab = NULL_RTX;
> +  if (use_after_return_class != -1)
> +    {
> +      rtx lab2 = gen_label_rtx ();
> +      char c = (char) ASAN_STACK_MAGIC_USE_AFTER_RET;
> +      int very_likely = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / 2000 - 1);
> +      emit_cmp_and_jump_insns (orig_base, base, EQ, NULL_RTX,
> +                              VOIDmode, 0, lab2, very_likely);
> +      shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
> +      set_mem_alias_set (shadow_mem, asan_shadow_set);
> +      mem = gen_rtx_MEM (ptr_mode, base);
> +      emit_move_insn (mem, gen_int_mode (ASAN_STACK_RETIRED_MAGIC, ptr_mode));
> +      if (use_after_return_class < 5
> +         && can_store_by_pieces (asan_frame_size >> ASAN_SHADOW_SHIFT,
> +                                 builtin_memset_read_str, &c, BITS_PER_UNIT,
> +                                 true))
> +       store_by_pieces (shadow_mem, asan_frame_size >> ASAN_SHADOW_SHIFT,
> +                        builtin_memset_read_str, &c, BITS_PER_UNIT, true, 0);
> +      else if (use_after_return_class >= 5
> +              || !set_storage_via_setmem (shadow_mem,
> +                                          GEN_INT (asan_frame_size
> +                                                   >> ASAN_SHADOW_SHIFT),
> +                                          gen_int_mode (c, QImode),
> +                                          BITS_PER_UNIT, BITS_PER_UNIT,
> +                                          -1))
> +       {
> +         snprintf (buf, sizeof buf, "__asan_stack_free_%d",
> +                   use_after_return_class);
> +         ret = init_one_libfunc (buf);
> +         rtx addr = convert_memory_address (ptr_mode, base);
> +         rtx orig_addr = convert_memory_address (ptr_mode, orig_base);
> +         emit_library_call (ret, LCT_NORMAL, ptr_mode, 3, addr, ptr_mode,
> +                            GEN_INT (asan_frame_size),
> +                            TYPE_MODE (pointer_sized_int_node),
> +                            orig_addr, ptr_mode);
> +       }
> +      lab = gen_label_rtx ();
> +      emit_jump (lab);
> +      emit_label (lab2);
> +    }
> +
>    shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
>    set_mem_alias_set (shadow_mem, asan_shadow_set);
>    prev_offset = base_offset;
> @@ -1092,6 +1183,8 @@ asan_emit_stack_protection (rtx base, HO
>      }
>
>    do_pending_stack_adjust ();
> +  if (lab)
> +    emit_label (lab);
>
>    ret = get_insns ();
>    end_sequence ();
> --- gcc/asan.h.jj       2013-11-12 11:31:23.000000000 +0100
> +++ gcc/asan.h  2013-11-14 15:42:57.371998293 +0100
> @@ -23,7 +23,7 @@ along with GCC; see the file COPYING3.
>
>  extern void asan_function_start (void);
>  extern void asan_finish_file (void);
> -extern rtx asan_emit_stack_protection (rtx, HOST_WIDE_INT *, tree *, int);
> +extern rtx asan_emit_stack_protection (rtx, rtx, HOST_WIDE_INT *, tree *, int);
>  extern bool asan_protect_global (tree);
>  extern void initialize_sanitizer_builtins (void);
>
> @@ -48,8 +48,10 @@ extern alias_set_type asan_shadow_set;
>  #define ASAN_STACK_MAGIC_MIDDLE                0xf2
>  #define ASAN_STACK_MAGIC_RIGHT         0xf3
>  #define ASAN_STACK_MAGIC_PARTIAL       0xf4
> +#define ASAN_STACK_MAGIC_USE_AFTER_RET 0xf5
>
> -#define ASAN_STACK_FRAME_MAGIC 0x41b58ab3
> +#define ASAN_STACK_FRAME_MAGIC         0x41b58ab3
> +#define ASAN_STACK_RETIRED_MAGIC       0x45e0360e
>
>  /* Return true if DECL should be guarded on the stack.  */
>
>
>
>         Jakub

On Thu, Nov 14, 2013 at 09:56:36PM +0400, Konstantin Serebryany wrote:
> I thought about alignment but did not reflect it anywhere in the
> interface/comments.
> The alignment should be min(4096, N), which is enough for most purposes.

You mean max(4096, N), right?  And, what exactly is N?  1 << (class_id + 6)?

> But let me double-check (and add a CHECK) tomorrow.

	Jakub

On Thu, Nov 14, 2013 at 07:08:17PM +0100, Jakub Jelinek wrote:
> On Thu, Nov 14, 2013 at 09:56:36PM +0400, Konstantin Serebryany wrote:
> > I thought about alignment but did not reflect it anywhere in the
> > interface/comments.
> > The alignment should be min(4096, N), which is enough for most purposes.
> 
> You mean max(4096, N), right?

Oops, of course min.

> And, what exactly is N?  1 << (class_id + 6)?

But this is valid.

	Jakub

On Thu, Nov 14, 2013 at 10:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Nov 14, 2013 at 07:08:17PM +0100, Jakub Jelinek wrote:
>> On Thu, Nov 14, 2013 at 09:56:36PM +0400, Konstantin Serebryany wrote:
>> > I thought about alignment but did not reflect it anywhere in the
>> > interface/comments.
>> > The alignment should be min(4096, N), which is enough for most purposes.
>>
>> You mean max(4096, N), right?
>
> Oops, of course min.
>
>> And, what exactly is N?  1 << (class_id + 6)?

Right.
The minimal allocation unit is 64-aligned 64-bytes.
Then 128-aligned 128-bytes and so on up to 4096.
Then only 4096-alignment is guaranteed.

These are the chunks returned by  __asan_stack_malloc.
The compiler pads them with redzones which in clang (and IIRC in gcc)
are 32-aligned 32-bytes.

You raised a valid concern about 512-bit (64-byte) alignment.
Clang asan does not support that now (will just not add a redzone to
such variables).
Someday we'll implement adaptable redzones for stack (we did that for
globals and heap already) which will fix this.

--kcc


>
> But this is valid.
>
>         Jakub