On Fri, Nov 15, 2013 at 12:06:25PM +0400, Konstantin Serebryany wrote:
> On Thu, Nov 14, 2013 at 10:09 PM, Jakub Jelinek <ja...@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;
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  <ja...@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

Reply via email to