Re: [PATCH] asan: Fix up address sanitizer instrumentation of __builtin_alloca* if it can throw [PR104449]

[Richard Biener via Gcc-patches]

> Am 12.02.2022 um 10:08 schrieb Jakub Jelinek via Gcc-patches 
> <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> With -fstack-check=generic __builtin_alloca* can throw and the asan
> instrumentation of this builtin wasn't prepared for that case.
> The following patch fixes that by replacing the builtin with the
> replacement builtin and emitting any further insns on the fallthru
> edge.
> 
> I haven't touched the hwasan code which most likely suffers from the
> same problem.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard 
> 2022-02-12  Jakub Jelinek  <ja...@redhat.com>
> 
>    PR sanitizer/104449
>    * asan.cc: Include tree-eh.h.
>    (handle_builtin_alloca): Handle the case when __builtin_alloca or
>    __builtin_alloca_with_align can throw.
> 
>    * gcc.dg/asan/pr104449.c: New test.
>    * g++.dg/asan/pr104449.C: New test.
> 
> --- gcc/asan.cc.jj    2022-01-18 11:58:58.876992143 +0100
> +++ gcc/asan.cc    2022-02-11 19:09:39.752065877 +0100
> @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.
> #include "fnmatch.h"
> #include "tree-inline.h"
> #include "tree-ssa.h"
> +#include "tree-eh.h"
> 
> /* AddressSanitizer finds out-of-bounds and use-after-free bugs
>    with <2x slowdown on average.
> @@ -726,14 +727,24 @@ handle_builtin_alloca (gcall *call, gimp
>   gassign *g;
>   gcall *gg;
>   tree callee = gimple_call_fndecl (call);
> +  tree lhs = gimple_call_lhs (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;
> +  tree ptr_type = lhs ? TREE_TYPE (lhs) : ptr_type_node;
>   tree partial_size = NULL_TREE;
>   unsigned int align
>     = DECL_FUNCTION_CODE (callee) == BUILT_IN_ALLOCA
>       ? 0 : tree_to_uhwi (gimple_call_arg (call, 1));
> 
> +  bool throws = false;
> +  edge e = NULL;
> +  if (stmt_can_throw_internal (cfun, call))
> +    {
> +      if (!lhs)
> +    return;
> +      throws = true;
> +      e = find_fallthru_edge (gsi_bb (*iter)->succs);
> +    }
> +
>   if (hwasan_sanitize_allocas_p ())
>     {
>       gimple_seq stmts = NULL;
> @@ -852,29 +863,54 @@ handle_builtin_alloca (gcall *call, gimp
>              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);
> -  gsi_insert_before (iter, gg, GSI_SAME_STMT);
> +  if (throws)
> +    {
> +      gimple_call_set_lhs (call, NULL);
> +      gsi_replace (iter, gg, true);
> +    }
> +  else
> +    gsi_insert_before (iter, gg, GSI_SAME_STMT);
> 
>   /* 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));
> -  gsi_insert_before (iter, g, GSI_SAME_STMT);
> +  gimple_stmt_iterator gsi = gsi_none ();
> +  if (throws)
> +    {
> +      gsi_insert_on_edge_immediate (e, g);
> +      gsi = gsi_for_stmt (g);
> +    }
> +  else
> +    gsi_insert_before (iter, g, GSI_SAME_STMT);
>   tree new_alloca = gimple_assign_lhs (g);
> 
>   /* 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);
> -  gsi_insert_before (iter, gg, GSI_SAME_STMT);
> +  if (throws)
> +    gsi_insert_after (&gsi, gg, GSI_NEW_STMT);
> +  else
> +    gsi_insert_before (iter, gg, GSI_SAME_STMT);
> 
>   /* Save new_alloca_with_rz value into last_alloca to use it during
>      allocas unpoisoning.  */
>   g = gimple_build_assign (last_alloca, new_alloca_with_rz);
> -  gsi_insert_before (iter, g, GSI_SAME_STMT);
> +  if (throws)
> +    gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> +  else
> +    gsi_insert_before (iter, g, GSI_SAME_STMT);
> 
>   /* Finally, replace old alloca ptr with NEW_ALLOCA.  */
> -  replace_call_with_value (iter, new_alloca);
> +  if (throws)
> +    {
> +      g = gimple_build_assign (lhs, new_alloca);
> +      gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> +    }
> +  else
> +    replace_call_with_value (iter, new_alloca);
> }
> 
> /* Return the memory references contained in a gimple statement
> --- gcc/testsuite/gcc.dg/asan/pr104449.c.jj    2022-02-11 19:23:05.085974426 
> +0100
> +++ gcc/testsuite/gcc.dg/asan/pr104449.c    2022-02-11 19:26:20.537282682 
> +0100
> @@ -0,0 +1,12 @@
> +/* PR sanitizer/104449 */
> +/* { dg-do compile } */
> +/* { dg-options "-fexceptions -fsanitize=address -fstack-check=generic" } */
> +
> +void bar (int *);
> +
> +void
> +foo (void)
> +{
> +  int a[16];
> +  bar (a);
> +}
> --- gcc/testsuite/g++.dg/asan/pr104449.C.jj    2022-02-11 19:25:22.035088372 
> +0100
> +++ gcc/testsuite/g++.dg/asan/pr104449.C    2022-02-11 19:26:08.605447008 
> +0100
> @@ -0,0 +1,16 @@
> +// PR sanitizer/104449
> +// { dg-do compile }
> +// { dg-options "-fexceptions -fsanitize=address -fstack-check=generic" }
> +
> +void bar (int *);
> +struct A { A (); ~A (); };
> +
> +void
> +foo (int n)
> +{
> +  A b;
> +  {
> +    int a[n];
> +    bar (a);
> +  }
> +}
> 
>    Jakub
>