Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Jakub Jelinek via Gcc-patches
On Wed, Jun 10, 2020 at 01:14:59PM +0200, Martin Liška wrote:
> >From 4d2e0b1e87b08ec21fd82144f00d364687030706 Mon Sep 17 00:00:00 2001
> From: Martin Liska 
> 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  
> 
>   PR sanitizer/94910
>   * asan.c (asan_emit_stack_protection): Emit
>   also **SavedFlagPtr(FakeStack) = 0 in order to release
>   a stack frame.

Please adjust the ChangeLog too.

Ok with that change.

Jakub



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Martin Liška

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, ,
-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, ,
+  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)(_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 
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  

	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, ,
   BITS_PER_UNIT, true))
-	store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
-			 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, ,
+			   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



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Jakub Jelinek via Gcc-patches
On Wed, Jun 10, 2020 at 11:49:01AM +0200, Martin Liška wrote:
> - store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> -  BITS_PER_UNIT, true, RETURN_BEGIN);
> + {
> +   /* Emit:
> +memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
> +**SavedFlagPtr(FakeStack) = 0

SavedFlagPtr has two arguments, doesn't it?

> +   */
> +   store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> +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.

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

This adds offset to it and changes mode to ptr_mode.  So,
mem is now *(ptr_mode)(_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?
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.

> +   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



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Martin Liška

On 6/10/20 10:42 AM, Jakub Jelinek wrote:

E.g. we just shouldn't reuse
MEMs (even after adjusting them) from different indirection levels because
we risk some attributes (alias set, MEM_EXPR, whatever else) will stay
around from the different indirection level.


All right, what about the updated patch? I must confess that RTL instruction
emission is not my strength.

Martin
>From 16e46a532c059930887bc30f82c3054a75a5a56d Mon Sep 17 00:00:00 2001
From: Martin Liska 
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  

	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..232341f5c4b 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, ,
   BITS_PER_UNIT, true))
-	store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
-			 BITS_PER_UNIT, true, RETURN_BEGIN);
+	{
+	  /* Emit:
+	   memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+	   **SavedFlagPtr(FakeStack) = 0
+	  */
+	  store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+			   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, ptr_mode, offset);
+	  rtx addr = gen_reg_rtx (ptr_mode);
+	  emit_move_insn (addr, mem);
+	  mem = gen_rtx_MEM (ptr_mode, addr);
+	  mem = adjust_address (mem, QImode, 0);
+	  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



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Jakub Jelinek via Gcc-patches
On Wed, Jun 10, 2020 at 10:24:59AM +0200, Martin Liška wrote:
> > > 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.
> 
> Jakub?

Even if so, just add that another level of indirection where it belongs,
but as I said, what you posted didn't feel right.  E.g. we just shouldn't reuse
MEMs (even after adjusting them) from different indirection levels because
we risk some attributes (alias set, MEM_EXPR, whatever else) will stay
around from the different indirection level.

Jakub



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-10 Thread Martin Liška

On 6/1/20 3:18 PM, Martin Liška wrote:

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, ,
    BITS_PER_UNIT, true))
-    store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
- BITS_PER_UNIT, true, RETURN_BEGIN);
+    {
+  /* Emit:
+   memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+   **SavedFlagPtr(FakeStack) = 0
+  */
+  store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+   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





Jakub?

Thanks,
Martin


Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-01 Thread Martin Liška

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, ,
  BITS_PER_UNIT, true))
-   store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
-BITS_PER_UNIT, true, RETURN_BEGIN);
+   {
+ /* Emit:
+  memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+  **SavedFlagPtr(FakeStack) = 0
+ */
+ store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+  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.s2020-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)
-   movq504(%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)
movq504(%rbx), %rax
movb$0, (%rax)
jmp .L3
.L2:

It does the same as my patch.
Martin



Jakub





Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-01 Thread Jakub Jelinek via Gcc-patches
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, ,
> BITS_PER_UNIT, true))
> - store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> -  BITS_PER_UNIT, true, RETURN_BEGIN);
> + {
> +   /* Emit:
> +memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
> +**SavedFlagPtr(FakeStack) = 0
> +   */
> +   store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
> +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.

Jakub



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-06-01 Thread Martin Liška

On 5/20/20 1:03 PM, Franz Sirl wrote:

Am 2020-05-19 um 21:05 schrieb Martin Liška:

Hi.

We make direct emission for asan_emit_stack_protection for smaller stacks.
That's fine but we're missing the piece that marks the stack as released
and we run out of pre-allocated stacks. I also included some stack-related
constants that were used in asan.c.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-05-19  Martin Liska  

 PR sanitizer/94910
 * asan.c (asan_emit_stack_protection): Emit
 also **SavedFlagPtr(FakeStack) = 0 in order to release
 a stack frame.
 * asan.h (ASAN_MIN_STACK_FRAME_SIZE_LOG): New.
 (ASAN_MAX_STACK_FRAME_SIZE_LOG): Likewise.
 (ASAN_MIN_STACK_FRAME_SIZE): Likewise.
 (ASAN_MAX_STACK_FRAME_SIZE): Likewise.
---
  gcc/asan.c | 26 ++
  gcc/asan.h |  8 
  2 files changed, 30 insertions(+), 4 deletions(-)




 >-  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase
 >+  if (asan_frame_size >= ASAN_MIN_STACK_FRAME_SIZE

Hi,

is the change from > to >= and from 32 to 64 for ASAN_MIN_STACK_FRAME_SIZE 
intentional? Just asking because it doesn't look obvious from Changelog or patch.
Also a few lines below the "5" in

   use_after_return_class = floor_log2 (asan_frame_size - 1) - 5;

looks like it may be related to ASAN_MIN_STACK_FRAME_SIZE_LOG.


Hello.

Thank you very much for the useful feedback. I really made the refactoring
in a wrong way.

I'm suggesting to only change the emission of asan_emit_stack_protection.
Tested locally with asan.exp file.

Ready for master?
Thanks,
Martin



regards,
Franz


>From 5d0c64b2f4028af3ed575934ecc0c3378cca3de1 Mon Sep 17 00:00:00 2001
From: Martin Liska 
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  

	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..e8d2a25ff79 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, ,
   BITS_PER_UNIT, true))
-	store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
-			 BITS_PER_UNIT, true, RETURN_BEGIN);
+	{
+	  /* Emit:
+	   memset(ShadowBase, kAsanStackAfterReturnMagic, ShadowSize);
+	   **SavedFlagPtr(FakeStack) = 0
+	  */
+	  store_by_pieces (shadow_mem, sz, builtin_memset_read_str, ,
+			   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);
+	}
   else if (use_after_return_class >= 5
 	   || !set_storage_via_setmem (shadow_mem,
 	   GEN_INT (sz),
-- 
2.26.2



Re: [PATCH] Add missing store in emission of asan_stack_free.

2020-05-20 Thread Franz Sirl

Am 2020-05-19 um 21:05 schrieb Martin Liška:

Hi.

We make direct emission for asan_emit_stack_protection for smaller stacks.
That's fine but we're missing the piece that marks the stack as released
and we run out of pre-allocated stacks. I also included some stack-related
constants that were used in asan.c.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-05-19  Martin Liska  

 PR sanitizer/94910
 * asan.c (asan_emit_stack_protection): Emit
 also **SavedFlagPtr(FakeStack) = 0 in order to release
 a stack frame.
 * asan.h (ASAN_MIN_STACK_FRAME_SIZE_LOG): New.
 (ASAN_MAX_STACK_FRAME_SIZE_LOG): Likewise.
 (ASAN_MIN_STACK_FRAME_SIZE): Likewise.
 (ASAN_MAX_STACK_FRAME_SIZE): Likewise.
---
  gcc/asan.c | 26 ++
  gcc/asan.h |  8 
  2 files changed, 30 insertions(+), 4 deletions(-)




>-  if (asan_frame_size > 32 && asan_frame_size <= 65536 && pbase
>+  if (asan_frame_size >= ASAN_MIN_STACK_FRAME_SIZE

Hi,

is the change from > to >= and from 32 to 64 for 
ASAN_MIN_STACK_FRAME_SIZE intentional? Just asking because it doesn't 
look obvious from Changelog or patch.

Also a few lines below the "5" in

  use_after_return_class = floor_log2 (asan_frame_size - 1) - 5;

looks like it may be related to ASAN_MIN_STACK_FRAME_SIZE_LOG.

regards,
Franz