diff mbox

[v8] PR middle-end/60281

Message ID 20140418042150.GA13430@ubuntu
State New
Headers show

Commit Message

林作健 April 18, 2014, 4:21 a.m. UTC 
Hi,
    Here is the patch after the Jakub's review, and Jakub helps with the
    coding style.

--

 * asan.c (asan_emit_stack_protection):
 Force the base to align to appropriate bits if STRICT_ALIGNMENT.  Set
 shadow_mem align to appropriate bits if STRICT_ALIGNMENT. 
 * cfgexpand.c
 (expand_stack_vars): Set base_align appropriately when asan is on.
 (expand_used_vars): Leave a space in the stack frame for alignment if
 STRICT_ALIGNMENT.

---
 gcc/ChangeLog   |  9 +++++++++
 gcc/asan.c      | 15 +++++++++++++++
 gcc/cfgexpand.c | 18 ++++++++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

Comments

林作健 April 18, 2014, 4:26 a.m. UTC | #1 
Hi Bernd,
a) On which target(s) did you boot-strap your patch?
    I just run it on x86, can't run it on ARM, because Android is not a
    posix system, nor a System V compatible system. And my code does not
    effect x86.

b) Did you run the testsuite?
    Yes, but again my code does not effect x86.

c) When you compare the test results with and without the patch, were there any regressions?
    Only the bug has gone. My app can run on my Android ARM system.

On Fri, Apr 18, 2014 at 12:21:50PM +0800, lin zuojian wrote:
>  Hi,
>     Here is the patch after the Jakub's review, and Jakub helps with the
>     coding style.
> 
> --
> 
>  * asan.c (asan_emit_stack_protection):
>  Force the base to align to appropriate bits if STRICT_ALIGNMENT.  Set
>  shadow_mem align to appropriate bits if STRICT_ALIGNMENT. 
>  * cfgexpand.c
>  (expand_stack_vars): Set base_align appropriately when asan is on.
>  (expand_used_vars): Leave a space in the stack frame for alignment if
>  STRICT_ALIGNMENT.
> 
> ---
>  gcc/ChangeLog   |  9 +++++++++
>  gcc/asan.c      | 15 +++++++++++++++
>  gcc/cfgexpand.c | 18 ++++++++++++++++--
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index da35be8..30a2b33 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2014-04-18  Lin Zuojian  <manjian2006@gmail.com>
> +       PR middle-end/60281
> +       * asan.c (asan_emit_stack_protection): Force the base to align to
> +       appropriate bits if STRICT_ALIGNMENT.  Set shadow_mem align to
> +       appropriate bits if STRICT_ALIGNMENT.
> +       * cfgexpand.c (expand_stack_vars): Set base_align appropriately
> +       when asan is on.
> +       (expand_used_vars): Leave a space in the stack frame for alignment
> +       if STRICT_ALIGNMENT.
>  2014-04-17  Jakub Jelinek  <jakub@redhat.com>
>  
>  	PR target/60847
> diff --git a/gcc/asan.c b/gcc/asan.c
> index 53992a8..28a476f 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>  	base_align_bias = ((asan_frame_size + alignb - 1)
>  			   & ~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
>      }
> +  /* Align base if target is STRICT_ALIGNMENT.  */
> +  if (STRICT_ALIGNMENT)
> +    base = expand_binop (Pmode, and_optab, base,
> +			 gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
> +					  << ASAN_SHADOW_SHIFT)
> +					 / BITS_PER_UNIT), Pmode), NULL_RTX,
> +			 1, OPTAB_DIRECT);
> +
>    if (use_after_return_class == -1 && pbase)
>      emit_move_insn (pbase, base);
> +
>    base = expand_binop (Pmode, add_optab, base,
>  		       gen_int_mode (base_offset - base_align_bias, Pmode),
>  		       NULL_RTX, 1, OPTAB_DIRECT);
> @@ -1097,6 +1106,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>  	      && (ASAN_RED_ZONE_SIZE >> ASAN_SHADOW_SHIFT) == 4);
>    shadow_mem = gen_rtx_MEM (SImode, shadow_base);
>    set_mem_alias_set (shadow_mem, asan_shadow_set);
> +  if (STRICT_ALIGNMENT)
> +    set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
>    prev_offset = base_offset;
>    for (l = length; l; l -= 2)
>      {
> @@ -1186,6 +1197,10 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>  
>    shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
>    set_mem_alias_set (shadow_mem, asan_shadow_set);
> +
> +  if (STRICT_ALIGNMENT)
> +    set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
> +
>    prev_offset = base_offset;
>    last_offset = base_offset;
>    last_size = 0;
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index b7f6360..14511e1 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -1013,10 +1013,19 @@ expand_stack_vars (bool (*pred) (size_t), struct stack_vars_data *data)
>  	      if (data->asan_base == NULL)
>  		data->asan_base = gen_reg_rtx (Pmode);
>  	      base = data->asan_base;
> +
> +	      if (!STRICT_ALIGNMENT)
> +		base_align = crtl->max_used_stack_slot_alignment;
> +	      else
> +		base_align = MAX (crtl->max_used_stack_slot_alignment,
> +				  GET_MODE_ALIGNMENT (SImode)
> +				  << ASAN_SHADOW_SHIFT);
>  	    }
>  	  else
> -	    offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
> -	  base_align = crtl->max_used_stack_slot_alignment;
> +	    {
> +	      offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
> +	      base_align = crtl->max_used_stack_slot_alignment;
> +	    }
>  	}
>        else
>  	{
> @@ -1845,6 +1854,11 @@ expand_used_vars (void)
>  	    = alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
>  	  data.asan_vec.safe_push (prev_offset);
>  	  data.asan_vec.safe_push (offset);
> +	  /* Leave space for alignment if STRICT_ALIGNMENT.  */
> +	  if (STRICT_ALIGNMENT)
> +	    alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
> +				      << ASAN_SHADOW_SHIFT)
> +				     / BITS_PER_UNIT, 1);
>  
>  	  var_end_seq
>  	    = asan_emit_stack_protection (virtual_stack_vars_rtx,
> -- 
> 1.8.3.2
> 
> --
> Regards
> lin zuojian
Bernd Edlinger April 18, 2014, 6:42 a.m. UTC | #2 
Hi Jakub,

I can take that task over and will boot-strap with all languages and run the
test suite on my armv7-linux-gnueabihf system.
But that will take until next week as it is currently occupied with other tests.


Can you please review Lin's latest patch and give your OK for check-in on trunk
and 4.9.1 branch?


Thanks
Bernd. 

On Fri, 18 Apr 2014 12:26:36, Lin Zuojian wrote:
>
> Hi Bernd,
> a) On which target(s) did you boot-strap your patch?
> I just run it on x86, can't run it on ARM, because Android is not a
> posix system, nor a System V compatible system. And my code does not
> effect x86.
>
> b) Did you run the testsuite?
> Yes, but again my code does not effect x86.
>
> c) When you compare the test results with and without the patch, were there any regressions?
> Only the bug has gone. My app can run on my Android ARM system.
>
> On Fri, Apr 18, 2014 at 12:21:50PM +0800, lin zuojian wrote:
>> Hi,
>> Here is the patch after the Jakub's review, and Jakub helps with the
>> coding style.
>>
>> --
>>
>> * asan.c (asan_emit_stack_protection):
>> Force the base to align to appropriate bits if STRICT_ALIGNMENT. Set
>> shadow_mem align to appropriate bits if STRICT_ALIGNMENT.
>> * cfgexpand.c
>> (expand_stack_vars): Set base_align appropriately when asan is on.
>> (expand_used_vars): Leave a space in the stack frame for alignment if
>> STRICT_ALIGNMENT.
>>
>> ---
>> gcc/ChangeLog | 9 +++++++++
>> gcc/asan.c | 15 +++++++++++++++
>> gcc/cfgexpand.c | 18 ++++++++++++++++--
>> 3 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index da35be8..30a2b33 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2014-04-18 Lin Zuojian <manjian2006@gmail.com>
>> + PR middle-end/60281
>> + * asan.c (asan_emit_stack_protection): Force the base to align to
>> + appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to
>> + appropriate bits if STRICT_ALIGNMENT.
>> + * cfgexpand.c (expand_stack_vars): Set base_align appropriately
>> + when asan is on.
>> + (expand_used_vars): Leave a space in the stack frame for alignment
>> + if STRICT_ALIGNMENT.
>> 2014-04-17 Jakub Jelinek <jakub@redhat.com>
>>
>> PR target/60847
>> diff --git a/gcc/asan.c b/gcc/asan.c
>> index 53992a8..28a476f 100644
>> --- a/gcc/asan.c
>> +++ b/gcc/asan.c
>> @@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>> base_align_bias = ((asan_frame_size + alignb - 1)
>> & ~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
>> }
>> + /* Align base if target is STRICT_ALIGNMENT. */
>> + if (STRICT_ALIGNMENT)
>> + base = expand_binop (Pmode, and_optab, base,
>> + gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
>> + << ASAN_SHADOW_SHIFT)
>> + / BITS_PER_UNIT), Pmode), NULL_RTX,
>> + 1, OPTAB_DIRECT);
>> +
>> if (use_after_return_class == -1 && pbase)
>> emit_move_insn (pbase, base);
>> +
>> base = expand_binop (Pmode, add_optab, base,
>> gen_int_mode (base_offset - base_align_bias, Pmode),
>> NULL_RTX, 1, OPTAB_DIRECT);
>> @@ -1097,6 +1106,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>> && (ASAN_RED_ZONE_SIZE>> ASAN_SHADOW_SHIFT) == 4);
>> shadow_mem = gen_rtx_MEM (SImode, shadow_base);
>> set_mem_alias_set (shadow_mem, asan_shadow_set);
>> + if (STRICT_ALIGNMENT)
>> + set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
>> prev_offset = base_offset;
>> for (l = length; l; l -= 2)
>> {
>> @@ -1186,6 +1197,10 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
>>
>> shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
>> set_mem_alias_set (shadow_mem, asan_shadow_set);
>> +
>> + if (STRICT_ALIGNMENT)
>> + set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
>> +
>> prev_offset = base_offset;
>> last_offset = base_offset;
>> last_size = 0;
>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
>> index b7f6360..14511e1 100644
>> --- a/gcc/cfgexpand.c
>> +++ b/gcc/cfgexpand.c
>> @@ -1013,10 +1013,19 @@ expand_stack_vars (bool (*pred) (size_t), struct stack_vars_data *data)
>> if (data->asan_base == NULL)
>> data->asan_base = gen_reg_rtx (Pmode);
>> base = data->asan_base;
>> +
>> + if (!STRICT_ALIGNMENT)
>> + base_align = crtl->max_used_stack_slot_alignment;
>> + else
>> + base_align = MAX (crtl->max_used_stack_slot_alignment,
>> + GET_MODE_ALIGNMENT (SImode)
>> + << ASAN_SHADOW_SHIFT);
>> }
>> else
>> - offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
>> - base_align = crtl->max_used_stack_slot_alignment;
>> + {
>> + offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
>> + base_align = crtl->max_used_stack_slot_alignment;
>> + }
>> }
>> else
>> {
>> @@ -1845,6 +1854,11 @@ expand_used_vars (void)
>> = alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
>> data.asan_vec.safe_push (prev_offset);
>> data.asan_vec.safe_push (offset);
>> + /* Leave space for alignment if STRICT_ALIGNMENT. */
>> + if (STRICT_ALIGNMENT)
>> + alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
>> + << ASAN_SHADOW_SHIFT)
>> + / BITS_PER_UNIT, 1);
>>
>> var_end_seq
>> = asan_emit_stack_protection (virtual_stack_vars_rtx,
>> --
>> 1.8.3.2
>>
>> --
>> Regards
>> lin zuojian
Jakub Jelinek April 18, 2014, 7:09 a.m. UTC | #3 
On Fri, Apr 18, 2014 at 12:21:50PM +0800, lin zuojian wrote:
>     Here is the patch after the Jakub's review, and Jakub helps with the
>     coding style.

> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2014-04-18  Lin Zuojian  <manjian2006@gmail.com>
> +       PR middle-end/60281

Extra line missing before the PR line.
Otherwise this is ok for trunk and for 4.9.1 after a while on the trunk (a
week or two).

Thanks.

	Jakub
林作健 April 18, 2014, 10:05 a.m. UTC | #4 
Hi Jakub,

On Fri, Apr 18, 2014 at 09:09:17AM +0200, Jakub Jelinek wrote:
> Extra line missing before the PR line.
Should I post PATCH v9 or someone helps adding one when committing
the patch?

--
Regards
lin zuojian
Jakub Jelinek April 18, 2014, 10:15 a.m. UTC | #5 
On Fri, Apr 18, 2014 at 06:05:35PM +0800, lin zuojian wrote:
> Hi Jakub,
> 
> On Fri, Apr 18, 2014 at 09:09:17AM +0200, Jakub Jelinek wrote:
> > Extra line missing before the PR line.
> Should I post PATCH v9 or someone helps adding one when committing
> the patch?

No need for that, that was meant for next time.  Anyway, I'll try to do an
arm scratch bootstrap/regtest of your patch and if all goes well, will check
it in (but as that may take a while, might do that only on Monday).

	Jakub
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index da35be8..30a2b33 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@ 
+2014-04-18  Lin Zuojian  <manjian2006@gmail.com>
+       PR middle-end/60281
+       * asan.c (asan_emit_stack_protection): Force the base to align to
+       appropriate bits if STRICT_ALIGNMENT.  Set shadow_mem align to
+       appropriate bits if STRICT_ALIGNMENT.
+       * cfgexpand.c (expand_stack_vars): Set base_align appropriately
+       when asan is on.
+       (expand_used_vars): Leave a space in the stack frame for alignment
+       if STRICT_ALIGNMENT.
 2014-04-17  Jakub Jelinek  <jakub@redhat.com>
 
 	PR target/60847
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..28a476f 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,17 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
 	base_align_bias = ((asan_frame_size + alignb - 1)
 			   & ~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
     }
+  /* Align base if target is STRICT_ALIGNMENT.  */
+  if (STRICT_ALIGNMENT)
+    base = expand_binop (Pmode, and_optab, base,
+			 gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
+					  << ASAN_SHADOW_SHIFT)
+					 / BITS_PER_UNIT), Pmode), NULL_RTX,
+			 1, OPTAB_DIRECT);
+
   if (use_after_return_class == -1 && pbase)
     emit_move_insn (pbase, base);
+
   base = expand_binop (Pmode, add_optab, base,
 		       gen_int_mode (base_offset - base_align_bias, Pmode),
 		       NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1106,8 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
 	      && (ASAN_RED_ZONE_SIZE >> ASAN_SHADOW_SHIFT) == 4);
   shadow_mem = gen_rtx_MEM (SImode, shadow_base);
   set_mem_alias_set (shadow_mem, asan_shadow_set);
+  if (STRICT_ALIGNMENT)
+    set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
   prev_offset = base_offset;
   for (l = length; l; l -= 2)
     {
@@ -1186,6 +1197,10 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
 
   shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
   set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+  if (STRICT_ALIGNMENT)
+    set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
   prev_offset = base_offset;
   last_offset = base_offset;
   last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index b7f6360..14511e1 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,19 @@  expand_stack_vars (bool (*pred) (size_t), struct stack_vars_data *data)
 	      if (data->asan_base == NULL)
 		data->asan_base = gen_reg_rtx (Pmode);
 	      base = data->asan_base;
+
+	      if (!STRICT_ALIGNMENT)
+		base_align = crtl->max_used_stack_slot_alignment;
+	      else
+		base_align = MAX (crtl->max_used_stack_slot_alignment,
+				  GET_MODE_ALIGNMENT (SImode)
+				  << ASAN_SHADOW_SHIFT);
 	    }
 	  else
-	    offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
-	  base_align = crtl->max_used_stack_slot_alignment;
+	    {
+	      offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
+	      base_align = crtl->max_used_stack_slot_alignment;
+	    }
 	}
       else
 	{
@@ -1845,6 +1854,11 @@  expand_used_vars (void)
 	    = alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
 	  data.asan_vec.safe_push (prev_offset);
 	  data.asan_vec.safe_push (offset);
+	  /* Leave space for alignment if STRICT_ALIGNMENT.  */
+	  if (STRICT_ALIGNMENT)
+	    alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
+				      << ASAN_SHADOW_SHIFT)
+				     / BITS_PER_UNIT, 1);
 
 	  var_end_seq
 	    = asan_emit_stack_protection (virtual_stack_vars_rtx,