* [RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. [not found] <CGME20170517122413eucas1p12f3a6e4e478c7fc2fddf513d2e3ea199@eucas1p1.samsung.com> @ 2017-05-17 12:25 ` Maxim Ostapenko 2017-05-29 7:28 ` Yuri Gribov 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-05-17 12:25 UTC (permalink / raw) To: GCC Patches [-- Attachment #1: Type: text/plain, Size: 1120 bytes --] Hi, this patch implements dynamic allocas/VLAs sanitization in ASan. Basically, this is implemented at compiler part in the following way: 1) For each __builtin_alloca{_with_align} increase its size and alignment to contain ASan redzones. 2) Poison redzones by calling __asan_alloca_poison(alloc_addr, size) ASan runtime library function. 3) Remember last allocated address into separate variable called 'last_alloca_addr'. This will be used to implement unpoisoning stuff. 4) On each stackrestore/return perform dynamic stack unpoisoning by calling __asan_allocas_unpoison(last_alloca_addr, restored_sp) library function. With this patch I was able to find two bugs in GCC itself [1], [2] as well as catch a bug in Radare2 [3] initially found by Clang + LibFuzzer. I've also managed to build Chromium but didn't find any errors there. Does this patch looks sensible for GCC? Any feedback/suggestions would be greatly appreciated. Thanks, -Maxim [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72765 [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80798 [3] https://github.com/radare/radare2/issues/6918 [-- Attachment #2: alloca-gcc-4.diff --] [-- Type: text/x-diff, Size: 23050 bytes --] gcc/ChangeLog: 2017-05-17 Maxim Ostapenko <m.ostapenko@samsung.com> * asan.c: Include gimple-fold.h. (get_last_alloca_addr): New function. (handle_builtin_stackrestore): Likewise. (handle_builtin_alloca): Likewise. (asan_emit_allocas_unpoison): Likewise. (get_mem_refs_of_builtin_call): Add new parameter, remove const quallifier from first paramerer. Handle BUILT_IN_ALLOCA, BUILT_IN_ALLOCA_WITH_ALIGN and BUILT_IN_STACK_RESTORE builtins. (instrument_builtin_call): Pass gimple iterator to get_mem_refs_of_builtin_call. (last_alloca_addr): New global. * asan.h (asan_emit_allocas_unpoison): Declare. * builtins.c (expand_asan_emit_allocas_unpoison): New function. (expand_builtin): Handle BUILT_IN_ASAN_ALLOCAS_UNPOISON. * cfgexpand.c (expand_used_vars): Call asan_emit_allocas_unpoison if function calls alloca. * gimple-fold.c (replace_call_with_value): Remove static keyword. * gimple-fold.h (replace_call_with_value): Declare. * internal-fn.c: Include asan.h. * sanitizer.def (BUILT_IN_ASAN_ALLOCA_POISON, BUILT_IN_ASAN_ALLOCAS_UNPOISON): New builtins. gcc/testsuite/ChangeLog: 2017-05-17 Maxim Ostapenko <m.ostapenko@samsung.com> * c-c++-common/asan/alloca_big_alignment.c: New test. * c-c++-common/asan/alloca_detect_custom_size.c: Likewise. * c-c++-common/asan/alloca_instruments_all_paddings.c: Likewise. * c-c++-common/asan/alloca_loop_unpoisoning.c: Likewise. * c-c++-common/asan/alloca_overflow_partial.c: Likewise. * c-c++-common/asan/alloca_overflow_right.c: Likewise. * c-c++-common/asan/alloca_safe_access.c: Likewise. * c-c++-common/asan/alloca_underflow_left.c: Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index bf564a4..4835db9 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "cfgloop.h" #include "gimple-builder.h" +#include "gimple-fold.h" #include "ubsan.h" #include "params.h" #include "builtins.h" @@ -245,6 +246,7 @@ along with GCC; see the file COPYING3. If not see static unsigned HOST_WIDE_INT asan_shadow_offset_value; static bool asan_shadow_offset_computed; static vec<char *> sanitized_sections; +static tree last_alloca_addr = NULL_TREE; /* Set of variable declarations that are going to be guarded by use-after-scope sanitizer. */ @@ -531,11 +533,166 @@ get_mem_ref_of_assignment (const gassign *assignment, return true; } +/* Return address of last allocated dynamic alloca. */ + +static tree +get_last_alloca_addr () +{ + if (last_alloca_addr) + return last_alloca_addr; + + gimple_seq seq = NULL; + gassign *g; + + last_alloca_addr = create_tmp_reg (ptr_type_node, "last_alloca_addr"); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, + build_int_cst (ptr_type_node, 0)); + gimple_seq_add_stmt_without_update (&seq, g); + + edge e = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)); + gsi_insert_seq_on_edge_immediate (e, seq); + return last_alloca_addr; +} + +/* Insert __asan_allocas_unpoison(top, bottom) call after + __builtin_stackrestore(new_sp) call. + The pseudocode of this routine should look like this: + __builtin_stackrestore(new_sp); + top = last_alloca_addr; + bot = virtual_dynamic_stack_rtx; + __asan_allocas_unpoison(top, bottom); + last_alloca_addr = new_sp; + We don't use new_sp as bot parameter because on some architectures + SP has non zero offset from dynamic stack area. Moreover, on some + architectures this offset (STACK_DYNAMIC_OFFSET) becomes known for each + particular function only after all callees were expanded to rtl. + The most noticable example is PowerPC{,64}, see + http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#DYNAM-STACK. +*/ + +static void +handle_builtin_stackrestore (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_stmt_iterator gsi = *iter; + gimple_seq seq = NULL; + tree last_alloca_addr = get_last_alloca_addr (); + tree restored_stack = gimple_call_arg (call, 0); + tree fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCAS_UNPOISON); + gimple *g = gimple_build_call (fn, 2, last_alloca_addr, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_after (&gsi, seq, GSI_SAME_STMT); +} + +/* Deploy and poison redzones around __builtin_alloca call. To do this, we + should replace this call with another one with changed parameters and + replace all its uses with new address, so + addr = __builtin_alloca (old_size, align); + is replaced by + new_size = old_size + additional_size; + tmp = __builtin_alloca (new_size, max(align, 32)) + addr = tmp + 32 (first 32 bytes are for the left redzone); + ADDITIONAL_SIZE is added to make new memory allocation contain not only + requested memory, but also left, partial and right redzones as well as some + additional space, required by alignment. */ + +static void +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_seq seq = NULL; + gassign *g; + gcall *gg; + gimple_stmt_iterator gsi = *iter; + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1; + + tree last_alloca_addr = get_last_alloca_addr (); + tree callee = gimple_call_fndecl (call); + tree old_size = gimple_call_arg (call, 0); + tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call)) + : ptr_type_node; + bool alloca_with_align + = (DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN); + unsigned int align + = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0; + + /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN + bytes of allocated space. */ + align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT); + + tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask); + tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE); + + /* misalign = size & (ASAN_RED_ZONE_SIZE - 1) + partial_size = ASAN_RED_ZONE_SIZE - misalign. */ + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR, + old_size, alloca_rz_mask); + gimple_seq_add_stmt_without_update (&seq, g); + tree misalign = gimple_assign_lhs (g); + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR, + redzone_size, misalign); + gimple_seq_add_stmt_without_update (&seq, g); + tree partial_size = gimple_assign_lhs (g); + + /* padding = align + ASAN_RED_ZONE_SIZE; + additional_size = padding + partial_size. */ + tree padding = build_int_cst (size_type_node, + align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE); + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, + partial_size, padding); + gimple_seq_add_stmt_without_update (&seq, g); + tree additional_size = gimple_assign_lhs (g); + + /* new_size = old_size + additional_size. */ + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size, + additional_size); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_size = gimple_assign_lhs (g); + + /* Build new __builtin_alloca call: + new_alloca_with_rz = __builtin_alloca(new_size, align). */ + tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN); + gg = gimple_build_call (fn, 2, new_size, + build_int_cst (size_type_node, align)); + tree new_alloca_with_rz = make_ssa_name (ptr_type, gg); + gimple_call_set_lhs (gg, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* new_alloca = new_alloca_with_rz + align. */ + g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR, + new_alloca_with_rz, + build_int_cst (size_type_node, + align / BITS_PER_UNIT)); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_alloca = gimple_assign_lhs (g); + + /* Replace old alloca ptr with NEW_ALLOCA. */ + replace_call_with_value (&gsi, new_alloca); + + /* Poison newly created alloca redzones: + __asan_alloca_poison(new_alloca, old_size). */ + fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON); + gg = gimple_build_call (fn, 2, new_alloca, old_size); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* Save new_alloca_with_rz value into last_alloca_addr to use it during + allocas unpoisoning. */ + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); +} + /* Return the memory references contained in a gimple statement representing a builtin call that has to do with memory access. */ static bool -get_mem_refs_of_builtin_call (const gcall *call, +get_mem_refs_of_builtin_call (gcall *call, asan_mem_ref *src0, tree *src0_len, bool *src0_is_store, @@ -546,7 +703,8 @@ get_mem_refs_of_builtin_call (const gcall *call, tree *dst_len, bool *dst_is_store, bool *dest_is_deref, - bool *intercepted_p) + bool *intercepted_p, + gimple_stmt_iterator *iter = NULL) { gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); @@ -605,6 +763,14 @@ get_mem_refs_of_builtin_call (const gcall *call, len = gimple_call_lhs (call); break; + case BUILT_IN_STACK_RESTORE: + handle_builtin_stackrestore (call, iter); + break; + + case BUILT_IN_ALLOCA_WITH_ALIGN: + case BUILT_IN_ALLOCA: + handle_builtin_alloca (call, iter); + break; /* And now the __atomic* and __sync builtins. These are handled differently from the classical memory memory access builtins above. */ @@ -1368,6 +1534,25 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, return insns; } +/* Emit __asan_allocas_unpoison(top, bot) call. The BASE parameter corresponds + to BOT argument, for TOP virtual_stack_dynamic_rtx is used. NEW_SEQUENCE + indicates whether we're emitting new instructions sequence or not. */ + +rtx_insn * +asan_emit_allocas_unpoison (rtx top, rtx bot) +{ + start_sequence (); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), bot, + TYPE_MODE (pointer_sized_int_node)); + + do_pending_stack_adjust (); + rtx_insn *insns = get_insns (); + end_sequence (); + return insns; +} + /* Return true if DECL, a global var, might be overridden and needs therefore a local alias. */ @@ -2002,7 +2187,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) &src0, &src0_len, &src0_is_store, &src1, &src1_len, &src1_is_store, &dest, &dest_len, &dest_is_store, - &dest_is_deref, &intercepted_p)) + &dest_is_deref, &intercepted_p, iter)) { if (dest_is_deref) { @@ -3188,6 +3373,7 @@ asan_instrument (void) if (shadow_ptr_types[0] == NULL_TREE) asan_init_shadow_ptr_types (); transform_statements (); + last_alloca_addr = NULL_TREE; return 0; } diff --git a/gcc/asan.h b/gcc/asan.h index 5766397..791271a 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -25,6 +25,7 @@ extern void asan_function_start (void); extern void asan_finish_file (void); extern rtx_insn *asan_emit_stack_protection (rtx, rtx, unsigned int, HOST_WIDE_INT *, tree *, int); +extern rtx_insn *asan_emit_allocas_unpoison (rtx, rtx); extern bool asan_protect_global (tree); extern void initialize_sanitizer_builtins (void); extern tree asan_dynamic_init_call (bool); diff --git a/gcc/builtins.c b/gcc/builtins.c index 4f6c9c4..dc42b3f 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -4962,6 +4962,20 @@ expand_builtin_alloca (tree exp, bool cannot_accumulate) return result; } +static rtx +expand_asan_emit_allocas_unpoison (tree exp) +{ + tree arg0 = CALL_EXPR_ARG (exp, 0); + rtx top = expand_expr (arg0, NULL_RTX, GET_MODE (virtual_stack_dynamic_rtx), + EXPAND_NORMAL); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), + virtual_stack_dynamic_rtx, + TYPE_MODE (pointer_sized_int_node)); + return ret; +} + /* Expand a call to bswap builtin in EXP. Return NULL_RTX if a normal call should be emitted rather than expanding the function in-line. If convenient, the result should be placed in TARGET. @@ -6759,6 +6773,12 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode, return target; break; + case BUILT_IN_ASAN_ALLOCAS_UNPOISON: + target = expand_asan_emit_allocas_unpoison (exp); + if (target) + return target; + break; + case BUILT_IN_STACK_SAVE: return expand_stack_save (); diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index e38c8e4..30a859c 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2241,6 +2241,10 @@ expand_used_vars (void) expand_stack_vars (NULL, &data); } + if ((flag_sanitize & SANITIZE_ADDRESS) && cfun->calls_alloca) + var_end_seq = asan_emit_allocas_unpoison (virtual_stack_dynamic_rtx, + virtual_stack_vars_rtx); + fini_vars_expansion (); /* If there were any artificial non-ignored vars without rtl diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index d12f9d0..0619224 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -570,7 +570,7 @@ gimplify_and_update_call_from_tree (gimple_stmt_iterator *si_p, tree expr) /* Replace the call at *GSI with the gimple value VAL. */ -static void +void replace_call_with_value (gimple_stmt_iterator *gsi, tree val) { gimple *stmt = gsi_stmt (*gsi); diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h index ad0b00d..2cee385 100644 --- a/gcc/gimple-fold.h +++ b/gcc/gimple-fold.h @@ -58,6 +58,7 @@ extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *); extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *); extern bool arith_code_with_undefined_signed_overflow (tree_code); extern gimple_seq rewrite_to_defined_overflow (gimple *); +extern void replace_call_with_value (gimple_stmt_iterator *, tree); /* gimple_build, functionally matching fold_buildN, outputs stmts int the provided sequence, matching and simplifying them on-the-fly. diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index 75fe027..eaaff90 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "dojump.h" #include "expr.h" +#include "asan.h" #include "ubsan.h" #include "recog.h" #include "builtins.h" diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 8b4acfc..91759a8 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -171,6 +171,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_POISON_STACK_MEMORY, DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_UNPOISON_STACK_MEMORY, "__asan_unpoison_stack_memory", BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCA_POISON, "__asan_alloca_poison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCAS_UNPOISON, "__asan_allocas_unpoison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) /* Thread Sanitizer */ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", diff --git a/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c new file mode 100644 index 0000000..54f03b8 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(128))); + assert(!((long) str & 127L)); + str[index] = '1'; // BOOM +} + +int main() { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_big_alignment.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_big_alignment.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c new file mode 100644 index 0000000..609dafe --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +struct A { + char a[3]; + int b[3]; +}; + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile struct A str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index].a[0] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_detect_custom_size.c:16|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_detect_custom_size.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c new file mode 100644 index 0000000..6b08c0b --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c @@ -0,0 +1,21 @@ +/* { dg-do run } */ + +#include "sanitizer/asan_interface.h" +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + char *q = (char *)__asan_region_is_poisoned((char *)str, 64); + assert(q && ((q - str) == index)); +} + +int main(int argc, char **argv) { + for (int i = 1; i < 33; ++i) + foo(i, i); + + for (int i = 1; i < 33; ++i) + foo(i, i); + + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c new file mode 100644 index 0000000..0ddadb9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c @@ -0,0 +1,34 @@ +/* { dg-do run } */ + +/* This testcase checks that allocas and VLAs inside loop are correctly unpoisoned. */ + +#include <assert.h> +#include <stdint.h> +#include <stdlib.h> +#include <stdio.h> +#include "sanitizer/asan_interface.h" + +void *top, *bot; +volatile int thirty_two = 32; + +__attribute__((noinline)) void foo(int len) { + char x; + top = &x; + volatile char array[len]; + assert(!((uintptr_t) array & 31L)); + alloca(len); + for (int i = 0; i < thirty_two; ++i) { + char array[i]; + bot = array; + /* Just to prevent optimization. */ + printf("%p\n", bot); + assert(!((uintptr_t) bot & 31L)); + } +} + +int main(int argc, char **argv) { + foo(thirty_two); + void *q = __asan_region_is_poisoned(bot, (char *)top - (char *)bot); + assert(!q); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c new file mode 100644 index 0000000..9f4d078 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_partial.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_partial.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c new file mode 100644 index 0000000..085fdae --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(33, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_right.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_right.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c new file mode 100644 index 0000000..92da1b2 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c @@ -0,0 +1,15 @@ +/* { dg-do run } */ + +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long)str & 31L)); + str[index] = '1'; +} + +int main(int argc, char **argv) { + foo(4, 5); + foo(39, 40); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c new file mode 100644 index 0000000..fe2abe1 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(-1, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_underflow_left.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_underflow_left.c.*(\n|\r\n|\r)" */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-05-17 12:25 ` [RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization Maxim Ostapenko @ 2017-05-29 7:28 ` Yuri Gribov 2017-06-06 7:32 ` [PING^2][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. Maxim Ostapenko 0 siblings, 1 reply; 15+ messages in thread From: Yuri Gribov @ 2017-05-29 7:28 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek On Wed, May 17, 2017 at 1:24 PM, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > Hi, > > this patch implements dynamic allocas/VLAs sanitization in ASan. Basically, > this is implemented at compiler part in the following way: > > 1) For each __builtin_alloca{_with_align} increase its size and alignment to > contain ASan redzones. > 2) Poison redzones by calling __asan_alloca_poison(alloc_addr, size) ASan > runtime library function. > 3) Remember last allocated address into separate variable called > 'last_alloca_addr'. This will be used to implement unpoisoning stuff. > 4) On each stackrestore/return perform dynamic stack unpoisoning by calling > __asan_allocas_unpoison(last_alloca_addr, restored_sp) library function. > > With this patch I was able to find two bugs in GCC itself [1], [2] as well > as catch a bug in Radare2 [3] initially found by Clang + LibFuzzer. > I've also managed to build Chromium but didn't find any errors there. > > Does this patch looks sensible for GCC? Any feedback/suggestions would be > greatly appreciated. > > Thanks, > -Maxim > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72765 > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80798 > [3] https://github.com/radare/radare2/issues/6918 Cc-ed sanitizer maintainers. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PING^2][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-05-29 7:28 ` Yuri Gribov @ 2017-06-06 7:32 ` Maxim Ostapenko 2017-06-13 12:11 ` [PING^3][RFC, " Maxim Ostapenko 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-06-06 7:32 UTC (permalink / raw) To: GCC Patches; +Cc: Jakub Jelinek, Marek Polacek, Yuri Gribov Hi, I would like to ping this patch: https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01374.html Thanks, -Maxim ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-06 7:32 ` [PING^2][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. Maxim Ostapenko @ 2017-06-13 12:11 ` Maxim Ostapenko 2017-06-13 13:15 ` Jakub Jelinek 2017-07-08 18:48 ` Andreas Schwab 0 siblings, 2 replies; 15+ messages in thread From: Maxim Ostapenko @ 2017-06-13 12:11 UTC (permalink / raw) To: GCC Patches; +Cc: Jakub Jelinek, Marek Polacek, Yuri Gribov [-- Attachment #1: Type: text/plain, Size: 150 bytes --] Hi, I would like to ping the following patch: https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01374.html Rebased version is attached. Thanks, -Maxim [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: alloca-gcc-6.diff --] [-- Type: text/x-diff; name="alloca-gcc-6.diff", Size: 23671 bytes --] gcc/ChangeLog: 2017-06-13 Maxim Ostapenko <m.ostapenko@samsung.com> * asan.c: Include gimple-fold.h. (get_last_alloca_addr): New function. (handle_builtin_stackrestore): Likewise. (handle_builtin_alloca): Likewise. (asan_emit_allocas_unpoison): Likewise. (get_mem_refs_of_builtin_call): Add new parameter, remove const quallifier from first paramerer. Handle BUILT_IN_ALLOCA, BUILT_IN_ALLOCA_WITH_ALIGN and BUILT_IN_STACK_RESTORE builtins. (instrument_builtin_call): Pass gimple iterator to get_mem_refs_of_builtin_call. (last_alloca_addr): New global. * asan.h (asan_emit_allocas_unpoison): Declare. * builtins.c (expand_asan_emit_allocas_unpoison): New function. (expand_builtin): Handle BUILT_IN_ASAN_ALLOCAS_UNPOISON. * cfgexpand.c (expand_used_vars): Call asan_emit_allocas_unpoison if function calls alloca. * gimple-fold.c (replace_call_with_value): Remove static keyword. * gimple-fold.h (replace_call_with_value): Declare. * internal-fn.c: Include asan.h. * sanitizer.def (BUILT_IN_ASAN_ALLOCA_POISON, BUILT_IN_ASAN_ALLOCAS_UNPOISON): New builtins. gcc/testsuite/ChangeLog: 2017-06-13 Maxim Ostapenko <m.ostapenko@samsung.com> * c-c++-common/asan/alloca_big_alignment.c: New test. * c-c++-common/asan/alloca_detect_custom_size.c: Likewise. * c-c++-common/asan/alloca_instruments_all_paddings.c: Likewise. * c-c++-common/asan/alloca_loop_unpoisoning.c: Likewise. * c-c++-common/asan/alloca_overflow_partial.c: Likewise. * c-c++-common/asan/alloca_overflow_right.c: Likewise. * c-c++-common/asan/alloca_safe_access.c: Likewise. * c-c++-common/asan/alloca_underflow_left.c: Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index bf564a4..4835db9 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "cfgloop.h" #include "gimple-builder.h" +#include "gimple-fold.h" #include "ubsan.h" #include "params.h" #include "builtins.h" @@ -245,6 +246,7 @@ along with GCC; see the file COPYING3. If not see static unsigned HOST_WIDE_INT asan_shadow_offset_value; static bool asan_shadow_offset_computed; static vec<char *> sanitized_sections; +static tree last_alloca_addr = NULL_TREE; /* Set of variable declarations that are going to be guarded by use-after-scope sanitizer. */ @@ -531,11 +533,166 @@ get_mem_ref_of_assignment (const gassign *assignment, return true; } +/* Return address of last allocated dynamic alloca. */ + +static tree +get_last_alloca_addr () +{ + if (last_alloca_addr) + return last_alloca_addr; + + gimple_seq seq = NULL; + gassign *g; + + last_alloca_addr = create_tmp_reg (ptr_type_node, "last_alloca_addr"); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, + build_int_cst (ptr_type_node, 0)); + gimple_seq_add_stmt_without_update (&seq, g); + + edge e = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)); + gsi_insert_seq_on_edge_immediate (e, seq); + return last_alloca_addr; +} + +/* Insert __asan_allocas_unpoison(top, bottom) call after + __builtin_stackrestore(new_sp) call. + The pseudocode of this routine should look like this: + __builtin_stackrestore(new_sp); + top = last_alloca_addr; + bot = virtual_dynamic_stack_rtx; + __asan_allocas_unpoison(top, bottom); + last_alloca_addr = new_sp; + We don't use new_sp as bot parameter because on some architectures + SP has non zero offset from dynamic stack area. Moreover, on some + architectures this offset (STACK_DYNAMIC_OFFSET) becomes known for each + particular function only after all callees were expanded to rtl. + The most noticable example is PowerPC{,64}, see + http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#DYNAM-STACK. +*/ + +static void +handle_builtin_stackrestore (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_stmt_iterator gsi = *iter; + gimple_seq seq = NULL; + tree last_alloca_addr = get_last_alloca_addr (); + tree restored_stack = gimple_call_arg (call, 0); + tree fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCAS_UNPOISON); + gimple *g = gimple_build_call (fn, 2, last_alloca_addr, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_after (&gsi, seq, GSI_SAME_STMT); +} + +/* Deploy and poison redzones around __builtin_alloca call. To do this, we + should replace this call with another one with changed parameters and + replace all its uses with new address, so + addr = __builtin_alloca (old_size, align); + is replaced by + new_size = old_size + additional_size; + tmp = __builtin_alloca (new_size, max(align, 32)) + addr = tmp + 32 (first 32 bytes are for the left redzone); + ADDITIONAL_SIZE is added to make new memory allocation contain not only + requested memory, but also left, partial and right redzones as well as some + additional space, required by alignment. */ + +static void +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_seq seq = NULL; + gassign *g; + gcall *gg; + gimple_stmt_iterator gsi = *iter; + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1; + + tree last_alloca_addr = get_last_alloca_addr (); + tree callee = gimple_call_fndecl (call); + tree old_size = gimple_call_arg (call, 0); + tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call)) + : ptr_type_node; + bool alloca_with_align + = (DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN); + unsigned int align + = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0; + + /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN + bytes of allocated space. */ + align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT); + + tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask); + tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE); + + /* misalign = size & (ASAN_RED_ZONE_SIZE - 1) + partial_size = ASAN_RED_ZONE_SIZE - misalign. */ + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR, + old_size, alloca_rz_mask); + gimple_seq_add_stmt_without_update (&seq, g); + tree misalign = gimple_assign_lhs (g); + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR, + redzone_size, misalign); + gimple_seq_add_stmt_without_update (&seq, g); + tree partial_size = gimple_assign_lhs (g); + + /* padding = align + ASAN_RED_ZONE_SIZE; + additional_size = padding + partial_size. */ + tree padding = build_int_cst (size_type_node, + align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE); + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, + partial_size, padding); + gimple_seq_add_stmt_without_update (&seq, g); + tree additional_size = gimple_assign_lhs (g); + + /* new_size = old_size + additional_size. */ + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size, + additional_size); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_size = gimple_assign_lhs (g); + + /* Build new __builtin_alloca call: + new_alloca_with_rz = __builtin_alloca(new_size, align). */ + tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN); + gg = gimple_build_call (fn, 2, new_size, + build_int_cst (size_type_node, align)); + tree new_alloca_with_rz = make_ssa_name (ptr_type, gg); + gimple_call_set_lhs (gg, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* new_alloca = new_alloca_with_rz + align. */ + g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR, + new_alloca_with_rz, + build_int_cst (size_type_node, + align / BITS_PER_UNIT)); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_alloca = gimple_assign_lhs (g); + + /* Replace old alloca ptr with NEW_ALLOCA. */ + replace_call_with_value (&gsi, new_alloca); + + /* Poison newly created alloca redzones: + __asan_alloca_poison(new_alloca, old_size). */ + fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON); + gg = gimple_build_call (fn, 2, new_alloca, old_size); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* Save new_alloca_with_rz value into last_alloca_addr to use it during + allocas unpoisoning. */ + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); +} + /* Return the memory references contained in a gimple statement representing a builtin call that has to do with memory access. */ static bool -get_mem_refs_of_builtin_call (const gcall *call, +get_mem_refs_of_builtin_call (gcall *call, asan_mem_ref *src0, tree *src0_len, bool *src0_is_store, @@ -546,7 +703,8 @@ get_mem_refs_of_builtin_call (const gcall *call, tree *dst_len, bool *dst_is_store, bool *dest_is_deref, - bool *intercepted_p) + bool *intercepted_p, + gimple_stmt_iterator *iter = NULL) { gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); @@ -605,6 +763,14 @@ get_mem_refs_of_builtin_call (const gcall *call, len = gimple_call_lhs (call); break; + case BUILT_IN_STACK_RESTORE: + handle_builtin_stackrestore (call, iter); + break; + + case BUILT_IN_ALLOCA_WITH_ALIGN: + case BUILT_IN_ALLOCA: + handle_builtin_alloca (call, iter); + break; /* And now the __atomic* and __sync builtins. These are handled differently from the classical memory memory access builtins above. */ @@ -1368,6 +1534,25 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, return insns; } +/* Emit __asan_allocas_unpoison(top, bot) call. The BASE parameter corresponds + to BOT argument, for TOP virtual_stack_dynamic_rtx is used. NEW_SEQUENCE + indicates whether we're emitting new instructions sequence or not. */ + +rtx_insn * +asan_emit_allocas_unpoison (rtx top, rtx bot) +{ + start_sequence (); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), bot, + TYPE_MODE (pointer_sized_int_node)); + + do_pending_stack_adjust (); + rtx_insn *insns = get_insns (); + end_sequence (); + return insns; +} + /* Return true if DECL, a global var, might be overridden and needs therefore a local alias. */ @@ -2002,7 +2187,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) &src0, &src0_len, &src0_is_store, &src1, &src1_len, &src1_is_store, &dest, &dest_len, &dest_is_store, - &dest_is_deref, &intercepted_p)) + &dest_is_deref, &intercepted_p, iter)) { if (dest_is_deref) { @@ -3188,6 +3373,7 @@ asan_instrument (void) if (shadow_ptr_types[0] == NULL_TREE) asan_init_shadow_ptr_types (); transform_statements (); + last_alloca_addr = NULL_TREE; return 0; } diff --git a/gcc/asan.h b/gcc/asan.h index 5766397..791271a 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -25,6 +25,7 @@ extern void asan_function_start (void); extern void asan_finish_file (void); extern rtx_insn *asan_emit_stack_protection (rtx, rtx, unsigned int, HOST_WIDE_INT *, tree *, int); +extern rtx_insn *asan_emit_allocas_unpoison (rtx, rtx); extern bool asan_protect_global (tree); extern void initialize_sanitizer_builtins (void); extern tree asan_dynamic_init_call (bool); diff --git a/gcc/builtins.c b/gcc/builtins.c index 30462ad..eb12c34 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -4962,6 +4962,20 @@ expand_builtin_alloca (tree exp) return result; } +static rtx +expand_asan_emit_allocas_unpoison (tree exp) +{ + tree arg0 = CALL_EXPR_ARG (exp, 0); + rtx top = expand_expr (arg0, NULL_RTX, GET_MODE (virtual_stack_dynamic_rtx), + EXPAND_NORMAL); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), + virtual_stack_dynamic_rtx, + TYPE_MODE (pointer_sized_int_node)); + return ret; +} + /* Expand a call to bswap builtin in EXP. Return NULL_RTX if a normal call should be emitted rather than expanding the function in-line. If convenient, the result should be placed in TARGET. @@ -6757,6 +6771,12 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode, return target; break; + case BUILT_IN_ASAN_ALLOCAS_UNPOISON: + target = expand_asan_emit_allocas_unpoison (exp); + if (target) + return target; + break; + case BUILT_IN_STACK_SAVE: return expand_stack_save (); diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index c1f8072..4ef2ca7 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2241,6 +2241,10 @@ expand_used_vars (void) expand_stack_vars (NULL, &data); } + if ((flag_sanitize & SANITIZE_ADDRESS) && cfun->calls_alloca) + var_end_seq = asan_emit_allocas_unpoison (virtual_stack_dynamic_rtx, + virtual_stack_vars_rtx); + fini_vars_expansion (); /* If there were any artificial non-ignored vars without rtl diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index d12f9d0..0619224 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -570,7 +570,7 @@ gimplify_and_update_call_from_tree (gimple_stmt_iterator *si_p, tree expr) /* Replace the call at *GSI with the gimple value VAL. */ -static void +void replace_call_with_value (gimple_stmt_iterator *gsi, tree val) { gimple *stmt = gsi_stmt (*gsi); diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h index ad0b00d..2cee385 100644 --- a/gcc/gimple-fold.h +++ b/gcc/gimple-fold.h @@ -58,6 +58,7 @@ extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *); extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *); extern bool arith_code_with_undefined_signed_overflow (tree_code); extern gimple_seq rewrite_to_defined_overflow (gimple *); +extern void replace_call_with_value (gimple_stmt_iterator *, tree); /* gimple_build, functionally matching fold_buildN, outputs stmts int the provided sequence, matching and simplifying them on-the-fly. diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index 75fe027..eaaff90 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "dojump.h" #include "expr.h" +#include "asan.h" #include "ubsan.h" #include "recog.h" #include "builtins.h" diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 8b4acfc..91759a8 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -171,6 +171,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_POISON_STACK_MEMORY, DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_UNPOISON_STACK_MEMORY, "__asan_unpoison_stack_memory", BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCA_POISON, "__asan_alloca_poison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCAS_UNPOISON, "__asan_allocas_unpoison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) /* Thread Sanitizer */ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", diff --git a/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c new file mode 100644 index 0000000..54f03b8 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(128))); + assert(!((long) str & 127L)); + str[index] = '1'; // BOOM +} + +int main() { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_big_alignment.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_big_alignment.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c new file mode 100644 index 0000000..609dafe --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +struct A { + char a[3]; + int b[3]; +}; + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile struct A str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index].a[0] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_detect_custom_size.c:16|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_detect_custom_size.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c new file mode 100644 index 0000000..6b08c0b --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c @@ -0,0 +1,21 @@ +/* { dg-do run } */ + +#include "sanitizer/asan_interface.h" +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + char *q = (char *)__asan_region_is_poisoned((char *)str, 64); + assert(q && ((q - str) == index)); +} + +int main(int argc, char **argv) { + for (int i = 1; i < 33; ++i) + foo(i, i); + + for (int i = 1; i < 33; ++i) + foo(i, i); + + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c new file mode 100644 index 0000000..0ddadb9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c @@ -0,0 +1,34 @@ +/* { dg-do run } */ + +/* This testcase checks that allocas and VLAs inside loop are correctly unpoisoned. */ + +#include <assert.h> +#include <stdint.h> +#include <stdlib.h> +#include <stdio.h> +#include "sanitizer/asan_interface.h" + +void *top, *bot; +volatile int thirty_two = 32; + +__attribute__((noinline)) void foo(int len) { + char x; + top = &x; + volatile char array[len]; + assert(!((uintptr_t) array & 31L)); + alloca(len); + for (int i = 0; i < thirty_two; ++i) { + char array[i]; + bot = array; + /* Just to prevent optimization. */ + printf("%p\n", bot); + assert(!((uintptr_t) bot & 31L)); + } +} + +int main(int argc, char **argv) { + foo(thirty_two); + void *q = __asan_region_is_poisoned(bot, (char *)top - (char *)bot); + assert(!q); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c new file mode 100644 index 0000000..9f4d078 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_partial.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_partial.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c new file mode 100644 index 0000000..085fdae --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(33, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_right.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_right.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c new file mode 100644 index 0000000..92da1b2 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c @@ -0,0 +1,15 @@ +/* { dg-do run } */ + +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long)str & 31L)); + str[index] = '1'; +} + +int main(int argc, char **argv) { + foo(4, 5); + foo(39, 40); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c new file mode 100644 index 0000000..fe2abe1 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(-1, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_underflow_left.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_underflow_left.c.*(\n|\r\n|\r)" */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-13 12:11 ` [PING^3][RFC, " Maxim Ostapenko @ 2017-06-13 13:15 ` Jakub Jelinek 2017-06-14 13:21 ` Maxim Ostapenko 2017-07-08 18:48 ` Andreas Schwab 1 sibling, 1 reply; 15+ messages in thread From: Jakub Jelinek @ 2017-06-13 13:15 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Marek Polacek, Yuri Gribov On Tue, Jun 13, 2017 at 03:11:41PM +0300, Maxim Ostapenko wrote: > @@ -531,11 +533,166 @@ get_mem_ref_of_assignment (const gassign *assignment, > return true; > } > > +/* Return address of last allocated dynamic alloca. */ > + > +static tree > +get_last_alloca_addr () > +{ > + if (last_alloca_addr) > + return last_alloca_addr; > + > + gimple_seq seq = NULL; > + gassign *g; > + > + last_alloca_addr = create_tmp_reg (ptr_type_node, "last_alloca_addr"); > +/* Insert __asan_allocas_unpoison(top, bottom) call after > + __builtin_stackrestore(new_sp) call. s/stackrestore/stack_restore/, that is how the builtin is called, right? Also, please put a space before ( even in comments. > +static void > +handle_builtin_stackrestore (gcall *call, gimple_stmt_iterator *iter) Again, stack_restore > + bool alloca_with_align > + = (DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN); Unnecessary ()s around the comparison? > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -2241,6 +2241,10 @@ expand_used_vars (void) > expand_stack_vars (NULL, &data); > } > > + if ((flag_sanitize & SANITIZE_ADDRESS) && cfun->calls_alloca) > + var_end_seq = asan_emit_allocas_unpoison (virtual_stack_dynamic_rtx, > + virtual_stack_vars_rtx); > + Doesn't this mean the old var_end_seq is lost because of this (in functions that call alloca, but also have addressable variables we asan instrument)? I'd think you need to append the sequences, or call asan_emit_allocas_unpoison with the var_end_seq as argument and insert it into the new sequence. Jakub ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-13 13:15 ` Jakub Jelinek @ 2017-06-14 13:21 ` Maxim Ostapenko 2017-06-14 17:34 ` Jakub Jelinek 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-06-14 13:21 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Marek Polacek, Yuri Gribov [-- Attachment #1: Type: text/plain, Size: 72 bytes --] Hi Jakub, thank you for your points. I've updated the patch. -Maxim [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: alloca-gcc-7.diff --] [-- Type: text/x-diff; name="alloca-gcc-7.diff", Size: 23797 bytes --] gcc/ChangeLog: 2017-06-14 Maxim Ostapenko <m.ostapenko@samsung.com> * asan.c: Include gimple-fold.h. (get_last_alloca_addr): New function. (handle_builtin_stackrestore): Likewise. (handle_builtin_alloca): Likewise. (asan_emit_allocas_unpoison): Likewise. (get_mem_refs_of_builtin_call): Add new parameter, remove const quallifier from first paramerer. Handle BUILT_IN_ALLOCA, BUILT_IN_ALLOCA_WITH_ALIGN and BUILT_IN_STACK_RESTORE builtins. (instrument_builtin_call): Pass gimple iterator to get_mem_refs_of_builtin_call. (last_alloca_addr): New global. * asan.h (asan_emit_allocas_unpoison): Declare. * builtins.c (expand_asan_emit_allocas_unpoison): New function. (expand_builtin): Handle BUILT_IN_ASAN_ALLOCAS_UNPOISON. * cfgexpand.c (expand_used_vars): Call asan_emit_allocas_unpoison if function calls alloca. * gimple-fold.c (replace_call_with_value): Remove static keyword. * gimple-fold.h (replace_call_with_value): Declare. * internal-fn.c: Include asan.h. * sanitizer.def (BUILT_IN_ASAN_ALLOCA_POISON, BUILT_IN_ASAN_ALLOCAS_UNPOISON): New builtins. gcc/testsuite/ChangeLog: 2017-06-14 Maxim Ostapenko <m.ostapenko@samsung.com> * c-c++-common/asan/alloca_big_alignment.c: New test. * c-c++-common/asan/alloca_detect_custom_size.c: Likewise. * c-c++-common/asan/alloca_instruments_all_paddings.c: Likewise. * c-c++-common/asan/alloca_loop_unpoisoning.c: Likewise. * c-c++-common/asan/alloca_overflow_partial.c: Likewise. * c-c++-common/asan/alloca_overflow_right.c: Likewise. * c-c++-common/asan/alloca_safe_access.c: Likewise. * c-c++-common/asan/alloca_underflow_left.c: Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index bf564a4..28e773a 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "cfgloop.h" #include "gimple-builder.h" +#include "gimple-fold.h" #include "ubsan.h" #include "params.h" #include "builtins.h" @@ -245,6 +246,7 @@ along with GCC; see the file COPYING3. If not see static unsigned HOST_WIDE_INT asan_shadow_offset_value; static bool asan_shadow_offset_computed; static vec<char *> sanitized_sections; +static tree last_alloca_addr = NULL_TREE; /* Set of variable declarations that are going to be guarded by use-after-scope sanitizer. */ @@ -531,11 +533,166 @@ get_mem_ref_of_assignment (const gassign *assignment, return true; } +/* Return address of last allocated dynamic alloca. */ + +static tree +get_last_alloca_addr () +{ + if (last_alloca_addr) + return last_alloca_addr; + + gimple_seq seq = NULL; + gassign *g; + + last_alloca_addr = create_tmp_reg (ptr_type_node, "last_alloca_addr"); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, + build_int_cst (ptr_type_node, 0)); + gimple_seq_add_stmt_without_update (&seq, g); + + edge e = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)); + gsi_insert_seq_on_edge_immediate (e, seq); + return last_alloca_addr; +} + +/* Insert __asan_allocas_unpoison (top, bottom) call after + __builtin_stack_restore (new_sp) call. + The pseudocode of this routine should look like this: + __builtin_stack_restore (new_sp); + top = last_alloca_addr; + bot = virtual_dynamic_stack_rtx; + __asan_allocas_unpoison (top, bottom); + last_alloca_addr = new_sp; + We don't use new_sp as bot parameter because on some architectures + SP has non zero offset from dynamic stack area. Moreover, on some + architectures this offset (STACK_DYNAMIC_OFFSET) becomes known for each + particular function only after all callees were expanded to rtl. + The most noticable example is PowerPC{,64}, see + http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#DYNAM-STACK. +*/ + +static void +handle_builtin_stack_restore (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_stmt_iterator gsi = *iter; + gimple_seq seq = NULL; + tree last_alloca_addr = get_last_alloca_addr (); + tree restored_stack = gimple_call_arg (call, 0); + tree fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCAS_UNPOISON); + gimple *g = gimple_build_call (fn, 2, last_alloca_addr, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, restored_stack); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_after (&gsi, seq, GSI_SAME_STMT); +} + +/* Deploy and poison redzones around __builtin_alloca call. To do this, we + should replace this call with another one with changed parameters and + replace all its uses with new address, so + addr = __builtin_alloca (old_size, align); + is replaced by + new_size = old_size + additional_size; + tmp = __builtin_alloca (new_size, max (align, 32)) + addr = tmp + 32 (first 32 bytes are for the left redzone); + ADDITIONAL_SIZE is added to make new memory allocation contain not only + requested memory, but also left, partial and right redzones as well as some + additional space, required by alignment. */ + +static void +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter) +{ + if (!iter) + return; + + gimple_seq seq = NULL; + gassign *g; + gcall *gg; + gimple_stmt_iterator gsi = *iter; + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1; + + tree last_alloca_addr = get_last_alloca_addr (); + tree callee = gimple_call_fndecl (call); + tree old_size = gimple_call_arg (call, 0); + tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call)) + : ptr_type_node; + bool alloca_with_align + = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN; + unsigned int align + = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0; + + /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN + bytes of allocated space. */ + align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT); + + tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask); + tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE); + + /* misalign = size & (ASAN_RED_ZONE_SIZE - 1) + partial_size = ASAN_RED_ZONE_SIZE - misalign. */ + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR, + old_size, alloca_rz_mask); + gimple_seq_add_stmt_without_update (&seq, g); + tree misalign = gimple_assign_lhs (g); + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR, + redzone_size, misalign); + gimple_seq_add_stmt_without_update (&seq, g); + tree partial_size = gimple_assign_lhs (g); + + /* padding = align + ASAN_RED_ZONE_SIZE; + additional_size = padding + partial_size. */ + tree padding = build_int_cst (size_type_node, + align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE); + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, + partial_size, padding); + gimple_seq_add_stmt_without_update (&seq, g); + tree additional_size = gimple_assign_lhs (g); + + /* new_size = old_size + additional_size. */ + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size, + additional_size); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_size = gimple_assign_lhs (g); + + /* Build new __builtin_alloca call: + new_alloca_with_rz = __builtin_alloca (new_size, align). */ + tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN); + gg = gimple_build_call (fn, 2, new_size, + build_int_cst (size_type_node, align)); + tree new_alloca_with_rz = make_ssa_name (ptr_type, gg); + gimple_call_set_lhs (gg, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* new_alloca = new_alloca_with_rz + align. */ + g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR, + new_alloca_with_rz, + build_int_cst (size_type_node, + align / BITS_PER_UNIT)); + gimple_seq_add_stmt_without_update (&seq, g); + tree new_alloca = gimple_assign_lhs (g); + + /* Replace old alloca ptr with NEW_ALLOCA. */ + replace_call_with_value (&gsi, new_alloca); + + /* Poison newly created alloca redzones: + __asan_alloca_poison (new_alloca, old_size). */ + fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON); + gg = gimple_build_call (fn, 2, new_alloca, old_size); + gimple_seq_add_stmt_without_update (&seq, gg); + + /* Save new_alloca_with_rz value into last_alloca_addr to use it during + allocas unpoisoning. */ + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz); + gimple_seq_add_stmt_without_update (&seq, g); + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); +} + /* Return the memory references contained in a gimple statement representing a builtin call that has to do with memory access. */ static bool -get_mem_refs_of_builtin_call (const gcall *call, +get_mem_refs_of_builtin_call (gcall *call, asan_mem_ref *src0, tree *src0_len, bool *src0_is_store, @@ -546,7 +703,8 @@ get_mem_refs_of_builtin_call (const gcall *call, tree *dst_len, bool *dst_is_store, bool *dest_is_deref, - bool *intercepted_p) + bool *intercepted_p, + gimple_stmt_iterator *iter = NULL) { gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); @@ -605,6 +763,14 @@ get_mem_refs_of_builtin_call (const gcall *call, len = gimple_call_lhs (call); break; + case BUILT_IN_STACK_RESTORE: + handle_builtin_stack_restore (call, iter); + break; + + case BUILT_IN_ALLOCA_WITH_ALIGN: + case BUILT_IN_ALLOCA: + handle_builtin_alloca (call, iter); + break; /* And now the __atomic* and __sync builtins. These are handled differently from the classical memory memory access builtins above. */ @@ -1368,6 +1534,28 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, return insns; } +/* Emit __asan_allocas_unpoison (top, bot) call. The BASE parameter corresponds + to BOT argument, for TOP virtual_stack_dynamic_rtx is used. NEW_SEQUENCE + indicates whether we're emitting new instructions sequence or not. */ + +rtx_insn * +asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) +{ + if (before) + push_to_sequence (before); + else + start_sequence (); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), bot, + TYPE_MODE (pointer_sized_int_node)); + + do_pending_stack_adjust (); + rtx_insn *insns = get_insns (); + end_sequence (); + return insns; +} + /* Return true if DECL, a global var, might be overridden and needs therefore a local alias. */ @@ -2002,7 +2190,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) &src0, &src0_len, &src0_is_store, &src1, &src1_len, &src1_is_store, &dest, &dest_len, &dest_is_store, - &dest_is_deref, &intercepted_p)) + &dest_is_deref, &intercepted_p, iter)) { if (dest_is_deref) { @@ -3188,6 +3376,7 @@ asan_instrument (void) if (shadow_ptr_types[0] == NULL_TREE) asan_init_shadow_ptr_types (); transform_statements (); + last_alloca_addr = NULL_TREE; return 0; } diff --git a/gcc/asan.h b/gcc/asan.h index 5766397..a6263b6 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -25,6 +25,7 @@ extern void asan_function_start (void); extern void asan_finish_file (void); extern rtx_insn *asan_emit_stack_protection (rtx, rtx, unsigned int, HOST_WIDE_INT *, tree *, int); +extern rtx_insn *asan_emit_allocas_unpoison (rtx, rtx, rtx_insn *); extern bool asan_protect_global (tree); extern void initialize_sanitizer_builtins (void); extern tree asan_dynamic_init_call (bool); diff --git a/gcc/builtins.c b/gcc/builtins.c index 30462ad..eb12c34 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -4962,6 +4962,20 @@ expand_builtin_alloca (tree exp) return result; } +static rtx +expand_asan_emit_allocas_unpoison (tree exp) +{ + tree arg0 = CALL_EXPR_ARG (exp, 0); + rtx top = expand_expr (arg0, NULL_RTX, GET_MODE (virtual_stack_dynamic_rtx), + EXPAND_NORMAL); + rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, + TYPE_MODE (pointer_sized_int_node), + virtual_stack_dynamic_rtx, + TYPE_MODE (pointer_sized_int_node)); + return ret; +} + /* Expand a call to bswap builtin in EXP. Return NULL_RTX if a normal call should be emitted rather than expanding the function in-line. If convenient, the result should be placed in TARGET. @@ -6757,6 +6771,12 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode, return target; break; + case BUILT_IN_ASAN_ALLOCAS_UNPOISON: + target = expand_asan_emit_allocas_unpoison (exp); + if (target) + return target; + break; + case BUILT_IN_STACK_SAVE: return expand_stack_save (); diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index c1f8072..b4450d4 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -2241,6 +2241,11 @@ expand_used_vars (void) expand_stack_vars (NULL, &data); } + if ((flag_sanitize & SANITIZE_ADDRESS) && cfun->calls_alloca) + var_end_seq = asan_emit_allocas_unpoison (virtual_stack_dynamic_rtx, + virtual_stack_vars_rtx, + var_end_seq); + fini_vars_expansion (); /* If there were any artificial non-ignored vars without rtl diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index d12f9d0..0619224 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -570,7 +570,7 @@ gimplify_and_update_call_from_tree (gimple_stmt_iterator *si_p, tree expr) /* Replace the call at *GSI with the gimple value VAL. */ -static void +void replace_call_with_value (gimple_stmt_iterator *gsi, tree val) { gimple *stmt = gsi_stmt (*gsi); diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h index ad0b00d..2cee385 100644 --- a/gcc/gimple-fold.h +++ b/gcc/gimple-fold.h @@ -58,6 +58,7 @@ extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *); extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *); extern bool arith_code_with_undefined_signed_overflow (tree_code); extern gimple_seq rewrite_to_defined_overflow (gimple *); +extern void replace_call_with_value (gimple_stmt_iterator *, tree); /* gimple_build, functionally matching fold_buildN, outputs stmts int the provided sequence, matching and simplifying them on-the-fly. diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index 75fe027..eaaff90 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "dojump.h" #include "expr.h" +#include "asan.h" #include "ubsan.h" #include "recog.h" #include "builtins.h" diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 8b4acfc..91759a8 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -171,6 +171,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_POISON_STACK_MEMORY, DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_UNPOISON_STACK_MEMORY, "__asan_unpoison_stack_memory", BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCA_POISON, "__asan_alloca_poison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_ALLOCAS_UNPOISON, "__asan_allocas_unpoison", + BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) /* Thread Sanitizer */ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", diff --git a/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c new file mode 100644 index 0000000..54f03b8 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_big_alignment.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(128))); + assert(!((long) str & 127L)); + str[index] = '1'; // BOOM +} + +int main() { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_big_alignment.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_big_alignment.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c new file mode 100644 index 0000000..609dafe --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_detect_custom_size.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +struct A { + char a[3]; + int b[3]; +}; + +volatile int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile struct A str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index].a[0] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_detect_custom_size.c:16|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_detect_custom_size.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c new file mode 100644 index 0000000..6b08c0b --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_instruments_all_paddings.c @@ -0,0 +1,21 @@ +/* { dg-do run } */ + +#include "sanitizer/asan_interface.h" +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + char *q = (char *)__asan_region_is_poisoned((char *)str, 64); + assert(q && ((q - str) == index)); +} + +int main(int argc, char **argv) { + for (int i = 1; i < 33; ++i) + foo(i, i); + + for (int i = 1; i < 33; ++i) + foo(i, i); + + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c new file mode 100644 index 0000000..0ddadb9 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c @@ -0,0 +1,34 @@ +/* { dg-do run } */ + +/* This testcase checks that allocas and VLAs inside loop are correctly unpoisoned. */ + +#include <assert.h> +#include <stdint.h> +#include <stdlib.h> +#include <stdio.h> +#include "sanitizer/asan_interface.h" + +void *top, *bot; +volatile int thirty_two = 32; + +__attribute__((noinline)) void foo(int len) { + char x; + top = &x; + volatile char array[len]; + assert(!((uintptr_t) array & 31L)); + alloca(len); + for (int i = 0; i < thirty_two; ++i) { + char array[i]; + bot = array; + /* Just to prevent optimization. */ + printf("%p\n", bot); + assert(!((uintptr_t) bot & 31L)); + } +} + +int main(int argc, char **argv) { + foo(thirty_two); + void *q = __asan_region_is_poisoned(bot, (char *)top - (char *)bot); + assert(!q); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c new file mode 100644 index 0000000..9f4d078 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_partial.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(ten, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_partial.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_partial.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c new file mode 100644 index 0000000..085fdae --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_overflow_right.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(33, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_overflow_right.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_overflow_right.c.*(\n|\r\n|\r)" */ diff --git a/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c new file mode 100644 index 0000000..92da1b2 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_safe_access.c @@ -0,0 +1,15 @@ +/* { dg-do run } */ + +#include <assert.h> + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long)str & 31L)); + str[index] = '1'; +} + +int main(int argc, char **argv) { + foo(4, 5); + foo(39, 40); + return 0; +} diff --git a/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c new file mode 100644 index 0000000..fe2abe1 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/alloca_underflow_left.c @@ -0,0 +1,22 @@ +/* { dg-do run } */ +/* { dg-shouldfail "asan" } */ + +#include <assert.h> + +volatile const int ten = 10; + +__attribute__((noinline)) void foo(int index, int len) { + volatile char str[len] __attribute__((aligned(32))); + assert(!((long) str & 31L)); + str[index] = '1'; // BOOM +} + +int main(int argc, char **argv) { + foo(-1, ten); + return 0; +} + +/* { dg-output "WRITE of size 1 at 0x\[0-9a-f\]+ thread T0\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output " #0 0x\[0-9a-f\]+ +(in _*foo(\[^\n\r]*alloca_underflow_left.c:11|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*Address 0x\[0-9a-f\]+ is located in stack of thread T0.*(\n|\r\n|\r)" */ +/* { dg-output "\[^\n\r]*in foo.*alloca_underflow_left.c.*(\n|\r\n|\r)" */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-14 13:21 ` Maxim Ostapenko @ 2017-06-14 17:34 ` Jakub Jelinek 2017-06-16 7:34 ` Richard Biener 0 siblings, 1 reply; 15+ messages in thread From: Jakub Jelinek @ 2017-06-14 17:34 UTC (permalink / raw) To: Maxim Ostapenko, Richard Biener; +Cc: GCC Patches, Marek Polacek, Yuri Gribov Hi! So, I've tried to look at struct __attribute__((aligned (N))) S { char s[N]; }; void bar (struct S *, struct S *); void foo (int x) { struct S a; { struct S b[x]; bar (&a, &b[0]); } { struct S b[x + 4]; bar (&a, &b[0]); } } void baz (int x) { struct S a; struct S b[x]; bar (&a, &b[0]); } testcase at -O2 -fsanitize=address -DN=64 (and -DN=8) on x86_64. Even in *.optimized dump I'm seeing: _1 = (sizetype) x_4(D); # RANGE [0, 18446744073709551552] NONZERO 18446744073709551552 _2 = _1 * 64; # RANGE [0, 31] NONZERO 31 _24 = _2 & 31; # RANGE ~[65, 127] _19 = _2 + 128; # RANGE ~[65, 96] _27 = _19 - _24; _28 = __builtin_alloca_with_align (_27, 512); _29 = _28 + 64; __builtin___asan_alloca_poison (_29, _2); which seems to be unnecessary complicated, as _2 has nonzero mask of 0xffffffffffffffc0 trying to and it with 0x1f should yield certainly _24 = 0 and thus there is no need to subtract anything. I wonder if this is just because the asan1 pass is fairly late and say ccp isn't scheduled after it. The question is if trying to use gimple_build APIs instead of gimple_build_assign would help here (and whether we'd need some new match.pd rules to figure out that if you have SSA_NAME & constant and get_nonzero_bits on the SSA_NAME & constant is 0, then the result is 0) or not. Or you could just try to check get_nonzero_bits yourself and if all the bits you want to mask are clear, avoid the subtraction. Also, isn't the size used for the adjusted __builtin_alloca_with_align too large? If you need _2 initially, and alignment is 64 bytes, then you certainly need 64 bytes before (unless we want to go into too low-level backend details and say that we want to allocate ret + 32 as 64-byte aligned), but 64 bytes after it is too much, 32 bytes would be enough (there is no partial right zone in this case)? On Wed, Jun 14, 2017 at 04:21:48PM +0300, Maxim Ostapenko wrote: > +static void > +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter) > +{ > + if (!iter) > + return; > + > + gimple_seq seq = NULL; > + gassign *g; > + gcall *gg; > + gimple_stmt_iterator gsi = *iter; > + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1; > + > + tree last_alloca_addr = get_last_alloca_addr (); > + tree callee = gimple_call_fndecl (call); > + tree old_size = gimple_call_arg (call, 0); > + tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call)) > + : ptr_type_node; > + bool alloca_with_align > + = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN; > + unsigned int align > + = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0; > + > + /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN > + bytes of allocated space. */ > + align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT); > + > + tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask); > + tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE); > + > + /* misalign = size & (ASAN_RED_ZONE_SIZE - 1) > + partial_size = ASAN_RED_ZONE_SIZE - misalign. */ > + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR, > + old_size, alloca_rz_mask); > + gimple_seq_add_stmt_without_update (&seq, g); > + tree misalign = gimple_assign_lhs (g); > + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR, > + redzone_size, misalign); > + gimple_seq_add_stmt_without_update (&seq, g); > + tree partial_size = gimple_assign_lhs (g); > + > + /* padding = align + ASAN_RED_ZONE_SIZE; > + additional_size = padding + partial_size. */ > + tree padding = build_int_cst (size_type_node, > + align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE); > + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, > + partial_size, padding); > + gimple_seq_add_stmt_without_update (&seq, g); > + tree additional_size = gimple_assign_lhs (g); > + > + /* new_size = old_size + additional_size. */ > + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size, > + additional_size); > + gimple_seq_add_stmt_without_update (&seq, g); > + tree new_size = gimple_assign_lhs (g); > + > + /* Build new __builtin_alloca call: > + new_alloca_with_rz = __builtin_alloca (new_size, align). */ > + tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN); > + gg = gimple_build_call (fn, 2, new_size, > + build_int_cst (size_type_node, align)); > + tree new_alloca_with_rz = make_ssa_name (ptr_type, gg); > + gimple_call_set_lhs (gg, new_alloca_with_rz); > + gimple_seq_add_stmt_without_update (&seq, gg); > + > + /* new_alloca = new_alloca_with_rz + align. */ > + g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR, > + new_alloca_with_rz, > + build_int_cst (size_type_node, > + align / BITS_PER_UNIT)); > + gimple_seq_add_stmt_without_update (&seq, g); > + tree new_alloca = gimple_assign_lhs (g); > + > + /* Replace old alloca ptr with NEW_ALLOCA. */ > + replace_call_with_value (&gsi, new_alloca); > + > + /* Poison newly created alloca redzones: > + __asan_alloca_poison (new_alloca, old_size). */ > + fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON); > + gg = gimple_build_call (fn, 2, new_alloca, old_size); > + gimple_seq_add_stmt_without_update (&seq, gg); > + > + /* Save new_alloca_with_rz value into last_alloca_addr to use it during > + allocas unpoisoning. */ > + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz); > + gimple_seq_add_stmt_without_update (&seq, g); > + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); > +} > + Jakub ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-14 17:34 ` Jakub Jelinek @ 2017-06-16 7:34 ` Richard Biener 0 siblings, 0 replies; 15+ messages in thread From: Richard Biener @ 2017-06-16 7:34 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Maxim Ostapenko, GCC Patches, Marek Polacek, Yuri Gribov On Wed, 14 Jun 2017, Jakub Jelinek wrote: > Hi! > > So, I've tried to look at > struct __attribute__((aligned (N))) S { char s[N]; }; > > void bar (struct S *, struct S *); > > void > foo (int x) > { > struct S a; > { > struct S b[x]; > bar (&a, &b[0]); > } > { > struct S b[x + 4]; > bar (&a, &b[0]); > } > } > > void > baz (int x) > { > struct S a; > struct S b[x]; > bar (&a, &b[0]); > } > testcase at -O2 -fsanitize=address -DN=64 (and -DN=8) on x86_64. > Even in *.optimized dump I'm seeing: > _1 = (sizetype) x_4(D); > # RANGE [0, 18446744073709551552] NONZERO 18446744073709551552 > _2 = _1 * 64; > # RANGE [0, 31] NONZERO 31 > _24 = _2 & 31; > # RANGE ~[65, 127] > _19 = _2 + 128; > # RANGE ~[65, 96] > _27 = _19 - _24; > _28 = __builtin_alloca_with_align (_27, 512); > _29 = _28 + 64; > __builtin___asan_alloca_poison (_29, _2); > which seems to be unnecessary complicated, as _2 has nonzero > mask of 0xffffffffffffffc0 trying to and it with 0x1f should > yield certainly _24 = 0 and thus there is no need to subtract anything. > > I wonder if this is just because the asan1 pass is fairly late and say > ccp isn't scheduled after it. The question is if trying to use > gimple_build APIs instead of gimple_build_assign would help here > (and whether we'd need some new match.pd rules to figure out > that if you have SSA_NAME & constant and get_nonzero_bits on the > SSA_NAME & constant is 0, then the result is 0) or not. The gimple_build API at the moment mirrors the behavior of building a large GENERIC expr which means it will only match-and-simplify stmts currently building (actually not yet associated with any BB). So if you build _2 & 31 you get that expr folded with match.pd rules (not sure if there is any yet doing the desired simplification to _2 using get_nonzero_bits). If you are building a stmt at a time folding built stmts would get you the same result (but then using the gimple_build helpers is more powerful) > Or you could just try to check get_nonzero_bits yourself and if > all the bits you want to mask are clear, avoid the subtraction. > > Also, isn't the size used for the adjusted __builtin_alloca_with_align > too large? If you need _2 initially, and alignment is 64 bytes, > then you certainly need 64 bytes before (unless we want to go into too > low-level backend details and say that we want to allocate ret + 32 > as 64-byte aligned), but 64 bytes after it is too much, 32 bytes would be > enough (there is no partial right zone in this case)? > > On Wed, Jun 14, 2017 at 04:21:48PM +0300, Maxim Ostapenko wrote: > > +static void > > +handle_builtin_alloca (gcall *call, gimple_stmt_iterator *iter) > > +{ > > + if (!iter) > > + return; > > + > > + gimple_seq seq = NULL; > > + gassign *g; > > + gcall *gg; > > + gimple_stmt_iterator gsi = *iter; > > + const HOST_WIDE_INT redzone_mask = ASAN_RED_ZONE_SIZE - 1; > > + > > + tree last_alloca_addr = get_last_alloca_addr (); > > + tree callee = gimple_call_fndecl (call); > > + tree old_size = gimple_call_arg (call, 0); > > + tree ptr_type = gimple_call_lhs (call) ? TREE_TYPE (gimple_call_lhs (call)) > > + : ptr_type_node; > > + bool alloca_with_align > > + = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA_WITH_ALIGN; > > + unsigned int align > > + = alloca_with_align ? tree_to_uhwi (gimple_call_arg (call, 1)) : 0; > > + > > + /* If ALIGN > ASAN_RED_ZONE_SIZE, we embed left redzone into first ALIGN > > + bytes of allocated space. */ > > + align = MAX (align, ASAN_RED_ZONE_SIZE * BITS_PER_UNIT); > > + > > + tree alloca_rz_mask = build_int_cst (size_type_node, redzone_mask); > > + tree redzone_size = build_int_cst (size_type_node, ASAN_RED_ZONE_SIZE); > > + > > + /* misalign = size & (ASAN_RED_ZONE_SIZE - 1) > > + partial_size = ASAN_RED_ZONE_SIZE - misalign. */ > > + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), BIT_AND_EXPR, > > + old_size, alloca_rz_mask); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + tree misalign = gimple_assign_lhs (g); > > + g = gimple_build_assign (make_ssa_name (size_type_node, NULL), MINUS_EXPR, > > + redzone_size, misalign); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + tree partial_size = gimple_assign_lhs (g); > > + > > + /* padding = align + ASAN_RED_ZONE_SIZE; > > + additional_size = padding + partial_size. */ > > + tree padding = build_int_cst (size_type_node, > > + align / BITS_PER_UNIT + ASAN_RED_ZONE_SIZE); > > + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, > > + partial_size, padding); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + tree additional_size = gimple_assign_lhs (g); > > + > > + /* new_size = old_size + additional_size. */ > > + g = gimple_build_assign (make_ssa_name (size_type_node), PLUS_EXPR, old_size, > > + additional_size); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + tree new_size = gimple_assign_lhs (g); > > + > > + /* Build new __builtin_alloca call: > > + new_alloca_with_rz = __builtin_alloca (new_size, align). */ > > + tree fn = builtin_decl_implicit (BUILT_IN_ALLOCA_WITH_ALIGN); > > + gg = gimple_build_call (fn, 2, new_size, > > + build_int_cst (size_type_node, align)); > > + tree new_alloca_with_rz = make_ssa_name (ptr_type, gg); > > + gimple_call_set_lhs (gg, new_alloca_with_rz); > > + gimple_seq_add_stmt_without_update (&seq, gg); > > + > > + /* new_alloca = new_alloca_with_rz + align. */ > > + g = gimple_build_assign (make_ssa_name (ptr_type), POINTER_PLUS_EXPR, > > + new_alloca_with_rz, > > + build_int_cst (size_type_node, > > + align / BITS_PER_UNIT)); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + tree new_alloca = gimple_assign_lhs (g); > > + > > + /* Replace old alloca ptr with NEW_ALLOCA. */ > > + replace_call_with_value (&gsi, new_alloca); > > + > > + /* Poison newly created alloca redzones: > > + __asan_alloca_poison (new_alloca, old_size). */ > > + fn = builtin_decl_implicit (BUILT_IN_ASAN_ALLOCA_POISON); > > + gg = gimple_build_call (fn, 2, new_alloca, old_size); > > + gimple_seq_add_stmt_without_update (&seq, gg); > > + > > + /* Save new_alloca_with_rz value into last_alloca_addr to use it during > > + allocas unpoisoning. */ > > + g = gimple_build_assign (last_alloca_addr, NOP_EXPR, new_alloca_with_rz); > > + gimple_seq_add_stmt_without_update (&seq, g); > > + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); > > +} > > + > > Jakub > > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-06-13 12:11 ` [PING^3][RFC, " Maxim Ostapenko 2017-06-13 13:15 ` Jakub Jelinek @ 2017-07-08 18:48 ` Andreas Schwab 2017-07-10 7:46 ` Maxim Ostapenko 1 sibling, 1 reply; 15+ messages in thread From: Andreas Schwab @ 2017-07-08 18:48 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov This breaks gcc.dg/asan/pr80168.c on aarch64 -mabi=ilp32: /opt/gcc/gcc-20170708/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal compiler error: in emit_library_call_value_1, at calls.c:4555 0x701577 emit_library_call_value_1 ../../gcc/calls.c:4554 0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type, machine_mode, int, ...) ../../gcc/calls.c:5159 0xb751d7 asan_emit_allocas_unpoison(rtx_def*, rtx_def*, rtx_insn*) ../../gcc/asan.c:1570 0x71cf1f expand_used_vars ../../gcc/cfgexpand.c:2245 0x71da07 execute ../../gcc/cfgexpand.c:6232 Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-08 18:48 ` Andreas Schwab @ 2017-07-10 7:46 ` Maxim Ostapenko 2017-07-11 13:52 ` Andreas Schwab 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-07-10 7:46 UTC (permalink / raw) To: Andreas Schwab; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov [-- Attachment #1: Type: text/plain, Size: 757 bytes --] Hi Andreas, On 08/07/17 21:48, Andreas Schwab wrote: > This breaks gcc.dg/asan/pr80168.c on aarch64 -mabi=ilp32: > > /opt/gcc/gcc-20170708/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal compiler error: in emit_library_call_value_1, at calls.c:4555 > 0x701577 emit_library_call_value_1 > ../../gcc/calls.c:4554 > 0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type, machine_mode, int, ...) > ../../gcc/calls.c:5159 > 0xb751d7 asan_emit_allocas_unpoison(rtx_def*, rtx_def*, rtx_insn*) > ../../gcc/asan.c:1570 > 0x71cf1f expand_used_vars > ../../gcc/cfgexpand.c:2245 > 0x71da07 execute > ../../gcc/cfgexpand.c:6232 could you check whether attached patch fixes the issue? -Maxim > > Andreas. > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: alloca-aarch64-ilp32-1.diff --] [-- Type: text/x-diff; name="alloca-aarch64-ilp32-1.diff", Size: 663 bytes --] diff --git a/gcc/asan.c b/gcc/asan.c index 95004d7..89c2731 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) else start_sequence (); rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + top = convert_memory_address (ptr_mode, top); + bot = convert_memory_address (ptr_mode, bot); ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, - TYPE_MODE (pointer_sized_int_node), bot, - TYPE_MODE (pointer_sized_int_node)); + ptr_mode, bot, ptr_mode); do_pending_stack_adjust (); rtx_insn *insns = get_insns (); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-10 7:46 ` Maxim Ostapenko @ 2017-07-11 13:52 ` Andreas Schwab 2017-07-11 14:25 ` Maxim Ostapenko 0 siblings, 1 reply; 15+ messages in thread From: Andreas Schwab @ 2017-07-11 13:52 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov On Jul 10 2017, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > diff --git a/gcc/asan.c b/gcc/asan.c > index 95004d7..89c2731 100644 > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) > else > start_sequence (); > rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); > + top = convert_memory_address (ptr_mode, top); > + bot = convert_memory_address (ptr_mode, bot); > ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, > - TYPE_MODE (pointer_sized_int_node), bot, > - TYPE_MODE (pointer_sized_int_node)); > + ptr_mode, bot, ptr_mode); There is another similar occurence: /opt/gcc/gcc-20170711/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal compiler error: in emit_library_call_value_1, at calls.c:4555 0x701577 emit_library_call_value_1 ../../gcc/calls.c:4554 0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type, machine_mode, int, ...) ../../gcc/calls.c:5159 0x6f2307 expand_asan_emit_allocas_unpoison ../../gcc/builtins.c:4978 0x6f2307 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) ../../gcc/builtins.c:6787 0x81fa6f expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) ../../gcc/expr.c:10841 0x716517 expand_expr ../../gcc/expr.h:276 0x716517 expand_call_stmt ../../gcc/cfgexpand.c:2664 0x716517 expand_gimple_stmt_1 ../../gcc/cfgexpand.c:3583 0x716517 expand_gimple_stmt ../../gcc/cfgexpand.c:3749 0x719077 expand_gimple_basic_block ../../gcc/cfgexpand.c:5753 0x71dfc7 execute ../../gcc/cfgexpand.c:6360 Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-11 13:52 ` Andreas Schwab @ 2017-07-11 14:25 ` Maxim Ostapenko 2017-07-11 14:57 ` Andreas Schwab 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-07-11 14:25 UTC (permalink / raw) To: Andreas Schwab; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov [-- Attachment #1: Type: text/plain, Size: 1813 bytes --] On 11/07/17 16:51, Andreas Schwab wrote: > On Jul 10 2017, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > >> diff --git a/gcc/asan.c b/gcc/asan.c >> index 95004d7..89c2731 100644 >> --- a/gcc/asan.c >> +++ b/gcc/asan.c >> @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) >> else >> start_sequence (); >> rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); >> + top = convert_memory_address (ptr_mode, top); >> + bot = convert_memory_address (ptr_mode, bot); >> ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, >> - TYPE_MODE (pointer_sized_int_node), bot, >> - TYPE_MODE (pointer_sized_int_node)); >> + ptr_mode, bot, ptr_mode); > There is another similar occurence: > > /opt/gcc/gcc-20170711/gcc/testsuite/gcc.dg/asan/pr80168.c:7:1: internal compiler error: in emit_library_call_value_1, at calls.c:4555 > 0x701577 emit_library_call_value_1 > ../../gcc/calls.c:4554 > 0x7068d7 emit_library_call_value(rtx_def*, rtx_def*, libcall_type, machine_mode, int, ...) > ../../gcc/calls.c:5159 > 0x6f2307 expand_asan_emit_allocas_unpoison > ../../gcc/builtins.c:4978 > 0x6f2307 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) > ../../gcc/builtins.c:6787 > 0x81fa6f expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) > ../../gcc/expr.c:10841 > 0x716517 expand_expr > ../../gcc/expr.h:276 > 0x716517 expand_call_stmt > ../../gcc/cfgexpand.c:2664 > 0x716517 expand_gimple_stmt_1 > ../../gcc/cfgexpand.c:3583 > 0x716517 expand_gimple_stmt > ../../gcc/cfgexpand.c:3749 > 0x719077 expand_gimple_basic_block > ../../gcc/cfgexpand.c:5753 > 0x71dfc7 execute > ../../gcc/cfgexpand.c:6360 Oh, I see. Does attached patch fix the issue? -Maxim > > Andreas. > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: alloca-aarch64-ilp32-2.diff --] [-- Type: text/x-diff; name="alloca-aarch64-ilp32-2.diff", Size: 1441 bytes --] gcc/ChangeLog: 2017-07-11 Maxim Ostapenko <m.ostapenko@samsung.com> * asan.c (asan_emit_allocas_unpoison): Use ptr_mode for arguments during expansion. * builtins.c (expand_asan_emit_allocas_unpoison): Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index 95004d7..89c2731 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) else start_sequence (); rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + top = convert_memory_address (ptr_mode, top); + bot = convert_memory_address (ptr_mode, bot); ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, - TYPE_MODE (pointer_sized_int_node), bot, - TYPE_MODE (pointer_sized_int_node)); + ptr_mode, bot, ptr_mode); do_pending_stack_adjust (); rtx_insn *insns = get_insns (); diff --git a/gcc/builtins.c b/gcc/builtins.c index 608993a..6437979 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp) EXPAND_NORMAL); rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, - TYPE_MODE (pointer_sized_int_node), - virtual_stack_dynamic_rtx, - TYPE_MODE (pointer_sized_int_node)); + ptr_mode, virtual_stack_dynamic_rtx, ptr_mode); return ret; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-11 14:25 ` Maxim Ostapenko @ 2017-07-11 14:57 ` Andreas Schwab 2017-07-11 16:27 ` Maxim Ostapenko 0 siblings, 1 reply; 15+ messages in thread From: Andreas Schwab @ 2017-07-11 14:57 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov On Jul 11 2017, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > diff --git a/gcc/builtins.c b/gcc/builtins.c > index 608993a..6437979 100644 > --- a/gcc/builtins.c > +++ b/gcc/builtins.c > @@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp) > EXPAND_NORMAL); > rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); > ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, > - TYPE_MODE (pointer_sized_int_node), > - virtual_stack_dynamic_rtx, > - TYPE_MODE (pointer_sized_int_node)); > + ptr_mode, virtual_stack_dynamic_rtx, ptr_mode); That doesn't work, same backtrace. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-11 14:57 ` Andreas Schwab @ 2017-07-11 16:27 ` Maxim Ostapenko 2017-07-12 16:58 ` Andreas Schwab 0 siblings, 1 reply; 15+ messages in thread From: Maxim Ostapenko @ 2017-07-11 16:27 UTC (permalink / raw) To: Andreas Schwab; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov [-- Attachment #1: Type: text/plain, Size: 902 bytes --] On 11/07/17 17:56, Andreas Schwab wrote: > On Jul 11 2017, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > >> diff --git a/gcc/builtins.c b/gcc/builtins.c >> index 608993a..6437979 100644 >> --- a/gcc/builtins.c >> +++ b/gcc/builtins.c >> @@ -4976,9 +4976,7 @@ expand_asan_emit_allocas_unpoison (tree exp) >> EXPAND_NORMAL); >> rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); >> ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, >> - TYPE_MODE (pointer_sized_int_node), >> - virtual_stack_dynamic_rtx, >> - TYPE_MODE (pointer_sized_int_node)); >> + ptr_mode, virtual_stack_dynamic_rtx, ptr_mode); > That doesn't work, same backtrace. > > Andreas. > Ok, I see, it seems that we need to add convert in expand_asan_emit_allocas_unpoison too. This patch seems to work for me on aarch64 -mabi=ilp32, could you check it as well? -Maxim [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: alloca-aarch64-ilp32-3.diff --] [-- Type: text/x-diff; name="alloca-aarch64-ilp32-3.diff", Size: 1704 bytes --] gcc/ChangeLog: 2017-07-11 Maxim Ostapenko <m.ostapenko@samsung.com> * asan.c (asan_emit_allocas_unpoison): Use ptr_mode for arguments during expansion. * builtins.c (expand_asan_emit_allocas_unpoison): Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index 95004d7..89c2731 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1567,9 +1567,10 @@ asan_emit_allocas_unpoison (rtx top, rtx bot, rtx_insn *before) else start_sequence (); rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); + top = convert_memory_address (ptr_mode, top); + bot = convert_memory_address (ptr_mode, bot); ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, - TYPE_MODE (pointer_sized_int_node), bot, - TYPE_MODE (pointer_sized_int_node)); + ptr_mode, bot, ptr_mode); do_pending_stack_adjust (); rtx_insn *insns = get_insns (); diff --git a/gcc/builtins.c b/gcc/builtins.c index 608993a..2deef72 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -4972,13 +4972,11 @@ static rtx expand_asan_emit_allocas_unpoison (tree exp) { tree arg0 = CALL_EXPR_ARG (exp, 0); - rtx top = expand_expr (arg0, NULL_RTX, GET_MODE (virtual_stack_dynamic_rtx), - EXPAND_NORMAL); + rtx top = expand_expr (arg0, NULL_RTX, ptr_mode, EXPAND_NORMAL); + rtx bot = convert_memory_address (ptr_mode, virtual_stack_dynamic_rtx); rtx ret = init_one_libfunc ("__asan_allocas_unpoison"); ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode, 2, top, - TYPE_MODE (pointer_sized_int_node), - virtual_stack_dynamic_rtx, - TYPE_MODE (pointer_sized_int_node)); + ptr_mode, bot, ptr_mode); return ret; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PING^3][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. 2017-07-11 16:27 ` Maxim Ostapenko @ 2017-07-12 16:58 ` Andreas Schwab 0 siblings, 0 replies; 15+ messages in thread From: Andreas Schwab @ 2017-07-12 16:58 UTC (permalink / raw) To: Maxim Ostapenko; +Cc: GCC Patches, Jakub Jelinek, Marek Polacek, Yuri Gribov On Jul 11 2017, Maxim Ostapenko <m.ostapenko@samsung.com> wrote: > Ok, I see, it seems that we need to add convert in > expand_asan_emit_allocas_unpoison too. This patch seems to work for me on > aarch64 -mabi=ilp32, could you check it as well? This fixes all regressions. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-07-12 16:58 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170517122413eucas1p12f3a6e4e478c7fc2fddf513d2e3ea199@eucas1p1.samsung.com>
2017-05-17 12:25 ` [RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization Maxim Ostapenko
2017-05-29 7:28 ` Yuri Gribov
2017-06-06 7:32 ` [PING^2][RFC, PATCH][ASAN] Implement dynamic allocas/VLAs sanitization. Maxim Ostapenko
2017-06-13 12:11 ` [PING^3][RFC, " Maxim Ostapenko
2017-06-13 13:15 ` Jakub Jelinek
2017-06-14 13:21 ` Maxim Ostapenko
2017-06-14 17:34 ` Jakub Jelinek
2017-06-16 7:34 ` Richard Biener
2017-07-08 18:48 ` Andreas Schwab
2017-07-10 7:46 ` Maxim Ostapenko
2017-07-11 13:52 ` Andreas Schwab
2017-07-11 14:25 ` Maxim Ostapenko
2017-07-11 14:57 ` Andreas Schwab
2017-07-11 16:27 ` Maxim Ostapenko
2017-07-12 16:58 ` Andreas Schwab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).