On Fri, Nov 15, 2013 at 2:10 PM, Jakub Jelinek <ja...@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 <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;
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 <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