On 6/1/20 2:52 PM, Jakub Jelinek wrote:
On Mon, Jun 01, 2020 at 02:28:51PM +0200, Martin Liška wrote:
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1598,8 +1598,24 @@ asan_emit_stack_protection (rtx base, rtx pbase, 
unsigned int alignb,
        if (use_after_return_class < 5
          && can_store_by_pieces (sz, builtin_memset_read_str, &c,
                                  BITS_PER_UNIT, true))
-       store_by_pieces (shadow_mem, sz, builtin_memset_read_str, &c,
-                        BITS_PER_UNIT, true, RETURN_BEGIN);
+       {
+         /* Emit:
+              memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+              **SavedFlagPtr(FakeStack) = 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 = adjust_address (mem, Pmode, offset);
+         mem = gen_rtx_MEM (ptr_mode, mem);
+         rtx tmp_reg = gen_reg_rtx (Pmode);
+         emit_move_insn (tmp_reg, mem);
+         mem = adjust_address (mem, QImode, 0);
+         emit_move_insn (mem, const0_rtx);

This doesn't look correct to me.
I'd think the first adjust_address should be
          mem = adjust_address (mem, ptr_mode, offset);
which will give you a MEM with ptr_mode which has SavedFlagPtr(FakeStack)
address, i.e. *SavedFlagPtr(FakeStack).
Next, you want to load that into some temporary, so e.g.
          rtx addr = gen_reg_rtx (ptr_mode);
          emit_move_insn (addr, mem);
next you need to convert that ptr_mode to Pmode if needed, so something like
          addr = convert_memory_address (Pmode, addr);
and finally:
          mem = gen_rtx_MEM (QImode, addr);
          emit_move_insn (mem, const0_rtx);
Completely untested.

This is not correct. With your suggestion I have:

int foo(int index)
{
  int a[100];
  return a[index];
}

$ diff -u before.s after.s
--- before.s    2020-06-01 15:15:22.634337654 +0200
+++ after.s     2020-06-01 15:16:32.205711511 +0200
@@ -81,8 +81,7 @@
        movq    %rdi, 2147450920(%rax)
        movq    %rsi, 2147450928(%rax)
        movq    %rdi, 2147450936(%rax)
-       movq    504(%rbx), %rax
-       movb    $0, (%rax)
+       movb    $0, 504(%rbx)
        jmp     .L3
 .L2:
        movq    $0, 2147450880(%rax)

There's missing one level of de-reference. Looking at clang:

        movq    %rsi, 2147450928(%rax)
        movq    %rdi, 2147450936(%rax)
        movq    504(%rbx), %rax
        movb    $0, (%rax)
        jmp     .L3
.L2:

It does the same as my patch.
Martin


        Jakub


Reply via email to