Re: libsanitizer merge from upstream r191666

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>]

On Fri, Nov 15, 2013 at 2:10 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Nov 15, 2013 at 12:06:25PM +0400, Konstantin Serebryany wrote:
>> 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.
>
> Ok.
>
>> 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.
>
> In GCC if no var actually requires 32-byte alignment, the redzones can
> be say only 16-byte or 8-byte aligned.
>
>> 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.
>
> GCC handles those, though I've found the patch I've posted yesterday
> actually broke that when using __asan_stack_malloc.  The problem is that
> in GCC what is guaranteed to be aligned properly is the end of the highest
> red zone, vars are allocated (sorted by decreasing alignment I think) from
> top to bottom.  So, say for:
> void bar (char *);
> void
> foo (void)
> {
>   char buf[64] __attribute__((aligned (64)));
>   bar (buf);
> }
> there would be 64-byte red zone after buf and 32-byte red zone before buf.
> Similarly for
> void
> baz (void)
> {
>   char buf[96] __attribute__((aligned (64)));
>   bar (buf);
> }
> there is 32-byte red zone after buf and 32-byte red zone before buf.
> The following updated patch, in addition of forcing not using
> __asan_stack_malloc_N if it is known not to have sufficient alignment,
> will for foo above instead of:
> base = __asan_stack_malloc_2 (160, top - 160);
> do:
> base = __asan_stack_malloc_2 (192, top - 192) + 32;

I afraid it actually wants the header (magic, descr, pc) to be in the
first 3 words in the
memory returned by __asan_stack_malloc_*
FakeStack::AddrIsInFakeStack(addr) returns the beginning of the allocated chunk
and then AsanThread::GetFrameNameByAddr expects the header to be there.

--kcc


> and to base store the magic word, descriptor string pointer etc.
> Is that ok with libsanitizer (i.e. that it doesn't assume that the magic
> word must be at what it returns)?  If not, I'd have to increase the
> bottom red zone, which I'd like to avoid if possible.
> For alignment <= 32 this isn't a problem, the distance between start of
> first red zone and end of last red zone is always divisible by 32.
>
> 2013-11-15  Jakub Jelinek  <jakub@redhat.com>
>
>         * cfgexpand.c (struct stack_vars_data): Add asan_base and asan_alignb
>         fields.
>         (expand_stack_vars): For -fsanitize=address, use (and set initially)
>         data->asan_base as base for vars and update asan_alignb.
>         (expand_used_vars): Initialize data.asan_base and data.asan_alignb.
>         Pass them to asan_emit_stack_protection.
>         * asan.c (asan_detect_stack_use_after_return): New variable.
>         (asan_emit_stack_protection): Add pbase and alignb arguments.
>         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-15 09:39:40.723074494 +0100
> +++ gcc/cfgexpand.c     2013-11-15 10:04:17.229465817 +0100
> @@ -879,6 +879,12 @@ 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;
> +
> +  /* Alignment needed for the Address Sanitizer protected automatic vars.  */
> +  unsigned int asan_alignb;
>  };
>
>  /* A subroutine of expand_used_vars.  Give each partition representative
> @@ -963,6 +969,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 +998,13 @@ 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);
> +             data->asan_alignb = MAX (data->asan_alignb, alignb);
> +             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
> @@ -1781,6 +1791,8 @@ expand_used_vars (void)
>
>        data.asan_vec = vNULL;
>        data.asan_decl_vec = vNULL;
> +      data.asan_base = NULL_RTX;
> +      data.asan_alignb = 0;
>
>        /* Reorder decls to be protected by iterating over the variables
>          array multiple times, and allocating out of each phase in turn.  */
> @@ -1813,8 +1825,10 @@ expand_used_vars (void)
>
>           var_end_seq
>             = asan_emit_stack_protection (virtual_stack_vars_rtx,
> +                                         data.asan_base,
> +                                         data.asan_alignb,
>                                           data.asan_vec.address (),
> -                                         data.asan_decl_vec. address (),
> +                                         data.asan_decl_vec.address (),
>                                           data.asan_vec.length ());
>         }
>
> --- gcc/asan.c.jj       2013-11-15 09:39:35.815099787 +0100
> +++ gcc/asan.c  2013-11-15 10:43:59.823217539 +0100
> @@ -227,6 +227,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.  */
>
> @@ -940,20 +943,26 @@ 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, unsigned int alignb,
> +                           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 base_offset = offsets[length - 1];
> +  HOST_WIDE_INT base_align_bias = 0, 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 ();
> @@ -983,10 +992,67 @@ 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;
> +      /* __asan_stack_malloc_N guarantees alignment
> +         N < 6 ? (64 << N) : 4096 bytes.  */
> +      if (alignb > (use_after_return_class < 6
> +                   ? (64U << use_after_return_class) : 4096U))
> +       use_after_return_class = -1;
> +      else if (alignb > ASAN_RED_ZONE_SIZE && (asan_frame_size & (alignb - 1)))
> +       base_align_bias = ((asan_frame_size + alignb - 1)
> +                          & ~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
> +    }
> +  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),
> +                      gen_int_mode (base_offset - base_align_bias, 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
> +                                             + base_align_bias),
> +                                    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_align_bias
> +                                                        - base_offset, Pmode),
> +                                          NULL_RTX, 1, OPTAB_DIRECT));
> +    }
>    mem = gen_rtx_MEM (ptr_mode, base);
> +  mem = adjust_address (mem, VOIDmode, base_align_bias);
>    emit_move_insn (mem, gen_int_mode (ASAN_STACK_FRAME_MAGIC, ptr_mode));
>    mem = adjust_address (mem, VOIDmode, GET_MODE_SIZE (ptr_mode));
>    emit_move_insn (mem, expand_normal (str_cst));
> @@ -1010,10 +1076,10 @@ 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 ()
> +                    + (base_align_bias >> ASAN_SHADOW_SHIFT));
>    gcc_assert (asan_shadow_set != -1
>               && (ASAN_RED_ZONE_SIZE >> ASAN_SHADOW_SHIFT) == 4);
>    shadow_mem = gen_rtx_MEM (SImode, shadow_base);
> @@ -1064,6 +1130,48 @@ 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);
> +      mem = adjust_address (mem, VOIDmode, base_align_bias);
> +      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 + base_align_bias),
> +                            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;
> @@ -1096,6 +1204,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-14 18:25:44.435248167 +0100
> +++ gcc/asan.h  2013-11-15 09:48:09.232452574 +0100
> @@ -23,7 +23,8 @@ 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, unsigned int, HOST_WIDE_INT *,
> +                                      tree *, int);
>  extern bool asan_protect_global (tree);
>  extern void initialize_sanitizer_builtins (void);
>
> @@ -48,8 +49,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