[PATCH] Fix -fsanitize=address VLA instrumentation (PR sanitizer/85230)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

As mentioned in the PR, we need to unpoison the red zones when leaving a
scope with VLA variable(s); this is done through __asan_allocas_unpoison
call, unfortunately it is called after the __builtin_stack_restore which
restores the stack pointer; now, if an interrupt comes in between the stack
restore and the __asan_allocas_unpoison call, the interrupt handler might
have some stack bytes marked as red zones in the shadow memory and might
diagnose sanitizing error even when there is none in the original program.

The following patch ought to fix this by swapping the two calls, so we first
unpoison and only after it is unpoisoned in shadow memory release the stack.
The second argument to the __asan_allocas_unpoison call is meant to
be virtual_dynamic_stack_rtx after the __builtin_stack_restore, i.e. the new
stack_pointer_rtx value + STACK_DYNAMIC_OFFSET (current_function_decl).
As the STACK_DYNAMIC_OFFSET value isn't known until the vregs pass, the code
used a hack where it ignored the second argument and replaced it by
virtual_dynamic_stack_rtx.  With the asan.c change below this doesn't work
anymore, because virtual_dynamic_stack_rtx aka stack_pointer_rtx +
STACK_DYNAMIC_OFFSET (current_function_decl) before the
__builtin_stack_restore is a different value.  The patch instead uses the
argument passed to the __asan_allocas_unpoison at GIMPLE time, which is the
same as passed to __builtin_stack_restore; this is the new stack_pointer_rtx
value after __builtin_stack_restore.  And, because we don't want that value,
but that + STACK_DYNAMIC_OFFSET (current_function_decl), we compute
arg1 + (virtual_dynamic_stack_rtx - stack_pointer_rtx) and let CSE/combiner
optimize it into arg1 (on targets like x86_64 where STACK_DYNAMIC_OFFSET can
be even 0 when not accumulating outgoing args or when that size is 0) or
arg1 + some_constant.

Bootstrapped on
{x86_64,i686,powerpc64,powerpc64le,aarch64,s390x,armv7hl}-linux, regtested
on {x86_64,i686,powerpc64,powerpc64le}-linux so far, but on the power* ones
on virtual address space size that isn't really supported (likely
https://github.com/google/sanitizers/issues/933#issuecomment-380058705
issue, so while nothing regresses there, pretty much all asan tests fail
there before and after the patch); also tested successfully with
asan.exp=alloca* on gcc110 and gcc112 on compile farm where it doesn't
suffer from the VA issue.  Ok if testing passes also on aarch64, s390x
and armv7hl?

2018-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/85230
	* asan.c (handle_builtin_stack_restore): Adjust comment.  Emit
	__asan_allocas_unpoison call and last_alloca_addr = new_sp before
	__builtin_stack_restore rather than after it.
	* builtins.c (expand_asan_emit_allocas_unpoison): Pass
	arg1 + (virtual_dynamic_stack_rtx - stack_pointer_rtx) as second
	argument instead of virtual_dynamic_stack_rtx.

--- gcc/asan.c.jj	2018-01-09 21:53:38.821577722 +0100
+++ gcc/asan.c	2018-04-12 13:22:59.166095523 +0200
@@ -554,14 +554,14 @@ get_last_alloca_addr ()
   return last_alloca_addr;
 }
 
-/* Insert __asan_allocas_unpoison (top, bottom) call after
+/* Insert __asan_allocas_unpoison (top, bottom) call before
    __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 = new_sp;
      __asan_allocas_unpoison (top, bot);
      last_alloca_addr = new_sp;
+     __builtin_stack_restore (new_sp);
    In general, we can'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
@@ -570,9 +570,8 @@ get_last_alloca_addr ()
    http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#DYNAM-STACK.
    To overcome the issue we use following trick: pass new_sp as a second
    parameter to __asan_allocas_unpoison and rewrite it during expansion with
-   virtual_dynamic_stack_rtx later in expand_asan_emit_allocas_unpoison
-   function.
-*/
+   new_sp + (virtual_dynamic_stack_rtx - sp) later in
+   expand_asan_emit_allocas_unpoison function.  */
 
 static void
 handle_builtin_stack_restore (gcall *call, gimple_stmt_iterator *iter)
@@ -584,9 +583,9 @@ handle_builtin_stack_restore (gcall *cal
   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, restored_stack);
-  gsi_insert_after (iter, g, GSI_NEW_STMT);
+  gsi_insert_before (iter, g, GSI_SAME_STMT);
   g = gimple_build_assign (last_alloca, restored_stack);
-  gsi_insert_after (iter, g, GSI_NEW_STMT);
+  gsi_insert_before (iter, g, GSI_SAME_STMT);
 }
 
 /* Deploy and poison redzones around __builtin_alloca call.  To do this, we
--- gcc/builtins.c.jj	2018-04-04 21:33:20.530639395 +0200
+++ gcc/builtins.c	2018-04-12 13:35:34.328395156 +0200
@@ -5068,18 +5068,24 @@ expand_builtin_alloca (tree exp)
   return result;
 }
 
-/* Emit a call to __asan_allocas_unpoison call in EXP.  Replace second argument
-   of the call with virtual_stack_dynamic_rtx because in asan pass we emit a
-   dummy value into second parameter relying on this function to perform the
-   change.  See motivation for this in comment to handle_builtin_stack_restore
-   function.  */
+/* Emit a call to __asan_allocas_unpoison call in EXP.  Add to second argument
+   of the call virtual_stack_dynamic_rtx - stack_pointer_rtx, which is the
+   STACK_DYNAMIC_OFFSET value.  See motivation for this in comment to
+   handle_builtin_stack_restore function.  */
 
 static rtx
 expand_asan_emit_allocas_unpoison (tree exp)
 {
   tree arg0 = CALL_EXPR_ARG (exp, 0);
+  tree arg1 = CALL_EXPR_ARG (exp, 1);
   rtx top = expand_expr (arg0, NULL_RTX, ptr_mode, EXPAND_NORMAL);
-  rtx bot = convert_memory_address (ptr_mode, virtual_stack_dynamic_rtx);
+  rtx bot = expand_expr (arg1, NULL_RTX, ptr_mode, EXPAND_NORMAL);
+  rtx off = expand_simple_binop (Pmode, MINUS, virtual_stack_dynamic_rtx,
+				 stack_pointer_rtx, NULL_RTX, 0,
+				 OPTAB_LIB_WIDEN);
+  off = convert_modes (ptr_mode, Pmode, off, 0);
+  bot = expand_simple_binop (ptr_mode, PLUS, bot, off, NULL_RTX, 0,
+			     OPTAB_LIB_WIDEN);
   rtx ret = init_one_libfunc ("__asan_allocas_unpoison");
   ret = emit_library_call_value (ret, NULL_RTX, LCT_NORMAL, ptr_mode,
 				 top, ptr_mode, bot, ptr_mode);

	Jakub