On 6/10/20 12:08 PM, Jakub Jelinek wrote:
On Wed, Jun 10, 2020 at 11:49:01AM +0200, Martin Liška wrote:
-       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

SavedFlagPtr has two arguments, doesn't it?

Good point, I copied that from 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
which has it wrong. Fixed that.


+         */
+         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);

So, mem here is a MEM into which we stored ASAN_STACK_RETIRED_MAGIC.

Ok, we should rather start from 'base'.


+         mem = adjust_address (mem, ptr_mode, offset);

This adds offset to it and changes mode to ptr_mode.  So,
mem is now *(ptr_mode)(&old_mem + offset)

+         rtx addr = gen_reg_rtx (ptr_mode);
+         emit_move_insn (addr, mem);

We load that value.

+         mem = gen_rtx_MEM (ptr_mode, addr);
+         mem = adjust_address (mem, QImode, 0);

And here I'm lost why you do that.  If you want to store a single
byte into what it points to, then why don't you just
        mem = gen_rtx_MEM (QImode, addr);
instead of the above two lines?

Because I'm not so much familiar with RTL ;)

adjust_address will return a MEM like the above, with offset not adjusted
(as the addition is 0) and mode changed to QImode, but there is no reason
not to create it already in QImode.

All right.
What about this?
Martin


+         emit_move_insn (mem, const0_rtx);
+       }
        else if (use_after_return_class >= 5
               || !set_storage_via_setmem (shadow_mem,
                                           GEN_INT (sz),
--
2.26.2



        Jakub


>From 4d2e0b1e87b08ec21fd82144f00d364687030706 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Tue, 19 May 2020 16:57:56 +0200
Subject: [PATCH] Add missing store in emission of asan_stack_free.

gcc/ChangeLog:

2020-05-19  Martin Liska  <mli...@suse.cz>

	PR sanitizer/94910
	* asan.c (asan_emit_stack_protection): Emit
	also **SavedFlagPtr(FakeStack) = 0 in order to release
	a stack frame.
---
 gcc/asan.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index c9872f1b007..e015fa3ec9b 100644
--- 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, 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);
+	  mem = gen_rtx_MEM (QImode, addr);
+	  emit_move_insn (mem, const0_rtx);
+	}
       else if (use_after_return_class >= 5
 	       || !set_storage_via_setmem (shadow_mem,
 					   GEN_INT (sz),
-- 
2.26.2

Reply via email to