On Thu, Jan 09, 2025 at 01:15:30AM +0100, Ilya Leoshkevich wrote:
> Bootstrapped and regtested on x86_64-redhat-linux.  Ok for master?
> 
> 
> 
> The FakeStack flag is not zeroed out when can_store_by_pieces()
> returns false.  Over time, this causes FakeStack::Allocate() to perform
> the maximum number of loop iterations, significantly slowing down the
> instrumented program.

Took me a while to construct a testcase where it makes a difference, but e.g.
void foo (int *, int *, int *, int *, int *, int *);

int
bar (void)
{
  int a[3], b[26];
  foo (a, b, 0, 0, 0, 0);
  return 0;
}

int
baz (void)
{
  int a[3], b[26], c[371], d[12], e[257], f[5];
  foo (a, b, c, d, e, f);
  return 0;
}
shows it on s390x with -O2 -fsanitize=address on bar but not baz (on x86_64
not on either).

> gcc/ChangeLog:
> 
>       * asan.cc (asan_emit_stack_protection): Always zero the flag
>       unless it is cleared by the __asan_stack_free_N() libcall.
> 
> Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
> ---
>  gcc/asan.cc | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/gcc/asan.cc b/gcc/asan.cc
> index 452a5a33327..da21d5e1008 100644
> --- a/gcc/asan.cc
> +++ b/gcc/asan.cc
> @@ -2167,27 +2167,16 @@ asan_emit_stack_protection (rtx base, rtx pbase, 
> unsigned int alignb,
>        mem = adjust_address (mem, VOIDmode, base_align_bias);
>        emit_move_insn (mem, gen_int_mode (ASAN_STACK_RETIRED_MAGIC, 
> ptr_mode));
>        unsigned HOST_WIDE_INT sz = asan_frame_size >> ASAN_SHADOW_SHIFT;
> +      bool asan_stack_free_emitted_p = false;
>        if (use_after_return_class < 5
>         && can_store_by_pieces (sz, builtin_memset_read_str, &c,
>                                 BITS_PER_UNIT, true))
>       {
>         /* Emit:
>              memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
> -            **SavedFlagPtr(FakeStack, class_id) = 0
>         */
>         store_by_pieces (shadow_mem, sz, builtin_memset_read_str, &c,
>                          BITS_PER_UNIT, true, RETURN_BEGIN);
> -
> -       unsigned HOST_WIDE_INT offset
> -         = (1 << (use_after_return_class + 6));
> -       offset -= GET_MODE_SIZE (ptr_mode);
> -       mem = gen_rtx_MEM (ptr_mode, base);
> -       mem = adjust_address (mem, ptr_mode, offset);
> -       rtx addr = gen_reg_rtx (ptr_mode);
> -       emit_move_insn (addr, mem);
> -       addr = convert_memory_address (Pmode, addr);
> -       mem = gen_rtx_MEM (QImode, addr);
> -       emit_move_insn (mem, const0_rtx);
>       }

This if body becomes single statement, so the {}s around it should be
dropped
and the comment and store_by_pieces call reindented by 2 columns to the
left.

> @@ -2205,6 +2194,23 @@ asan_emit_stack_protection (rtx base, rtx pbase, 
> unsigned int alignb,
>                            GEN_INT (asan_frame_size + base_align_bias),
>                            TYPE_MODE (pointer_sized_int_node),
>                            orig_addr, ptr_mode);
> +       asan_stack_free_emitted_p = true;
> +     }
> +      if (!asan_stack_free_emitted_p)
> +     {
> +       /* Emit:
> +            **SavedFlagPtr (FakeStack, class_id) = 0
> +       */
> +       unsigned HOST_WIDE_INT offset
> +         = (1 << (use_after_return_class + 6));
> +       offset -= GET_MODE_SIZE (ptr_mode);
> +       mem = gen_rtx_MEM (ptr_mode, base);
> +       mem = adjust_address (mem, ptr_mode, offset);
> +       rtx addr = gen_reg_rtx (ptr_mode);
> +       emit_move_insn (addr, mem);
> +       addr = convert_memory_address (Pmode, addr);
> +       mem = gen_rtx_MEM (QImode, addr);
> +       emit_move_insn (mem, const0_rtx);
>       }
>        lab = gen_label_rtx ();
>        emit_jump (lab);

Ok for trunk with that nit fixed.

        Jakub

Reply via email to