Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, I have my copyright mark signed and the process has completed. Now I am going to answer two more questions before my patch can be commited right? Did you copy any files or text written by someone else in these changes?” no [Which files have you changed so far, and which new files have you written so far?] gcc/asan.c gcc/ChangeLog gcc/cfgexpand.c Okay, you may review my patch again, if there is no problem, please commit it for me. -- Regards lin zuojian
RE: [PATCH v7?] PR middle-end/60281
Hi Lin, On Thu, 17 Apr 2014 22:29:14, Lin Zuojian wrote: Hi Bernd, I have my copyright mark signed and the process has completed. Now I am going to answer two more questions before my patch can be commited right? Did you copy any files or text written by someone else in these changes?” no [Which files have you changed so far, and which new files have you written so far?] gcc/asan.c gcc/ChangeLog gcc/cfgexpand.c Okay, you may review my patch again, if there is no problem, please commit it for me. -- Regards lin zuojian I am not sure if your patch was already approved by a global GCC reviewer. That is however absolutely necessary before it can be committed. I think it would be best to re-submit the latest version of your patch now, and ask a global reviewer for approval. The message should be sent to gcc-patches@gcc.gnu.org and contain the following information in addition to the proposed patch itself and the change-log entry: a) On which target(s) did you boot-strap your patch? b) Did you run the testsuite? c) When you compare the test results with and without the patch, were there any regressions? Regards Bernd.
Re: [PATCH v7] PR middle-end/60281
Hi, GCC changes(http://gcc.gnu.org/gcc-4.9/changes.html) says: AddressSanitizer, a fast memory error detector, is now available on ARM. But I test it,and find PR60281 is still there!That means the wrong shadow bytes will still populate using stm instructions. So,I don't think asan is available on ARM. Anyway I have signed the FSF copyright form.I hope my patch will be apply on newer GCC. -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, Post stations are not that 90's,and they charge.It took me $30 to post the file to USA.It's so inconvenient and expensive that I can't send a scaned version. -- Regards lin zuojian On Wed, Apr 09, 2014 at 06:47:56PM +0800, lin zuojian wrote: Hi Bernd, I am asking them if they would accept a scaned image version.Post station is so 90's -- Regards lin zuojian
RE: [PATCH v7?] PR middle-end/60281
Hi Lin, thanks for clarifying this. If you say you can't sign the FSF copyright assignment, we can't use your patch, I'm afraid. Well, I was curious how to proceed, because these unaligned stm instructions are also a problem under linux. The test cases don't fail, because the exception handler emulates these instructions, but that is quite annoying, because each time the test suite runs, there are many syslog entries complaining about unaligned stm instructions. Actually I had begun to work on a patch for this issue at about the same time when you posted your patch. As it looked like your patch was likely to be approved soon, I decided to wait for your patch. But now I think, maybe I should propose my alternative patch instead, if you don't mind. Regards Bernd.
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, Seem we are not talking the same problem.You should first make sure what has been going wrong first. And I will sign it. -- Regards lin zuojian
RE: [PATCH v7?] PR middle-end/60281
Hi Lin, Seem we are not talking the same problem.You should first make sure what has been going wrong first. Maybe I misunderstood your point. And I will sign it. -- Regards lin zuojian Ok, then please do it. Once you have signed it, and got the approval by a global GCC reviewer, I would be happy to assist you in committing that patch, if you like. Regards Bernd.
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, I am asking them if they would accept a scaned image version.Post station is so 90's -- Regards lin zuojian
RE: [PATCH v7?] PR middle-end/60281
Lin, are you still working on this? Thanks Bernd.
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, in a way,yes.I am testing it.Since I don't have a FSF copyright mark,and I have no idea what is that file saying,I can't commit my patch.If you are interested in it,please help me commit it. By the way,there is another way to work it out,and it has been mentioned in the previous(I don't know how previous) mail.That is to use unaligned instructions instead of aligned ones on the armv7 or above platform.That has to modify part of machine desc of arm platform.I can't tell you it's one of the define_peephole.I have no time to test this idea yet.If you are interested in that way,please test this one.It's always better when there is an option. -- Regards lin zuojian
Re: [PATCH v7] PR middle-end/60281
On Tue, Mar 04, 2014 at 11:11:45AM +0800, lin zuojian wrote:
Without aligning the asan stack base,this base will only 64-bit aligned in
ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits
aligned.
* 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.
There were still a couple of formatting issues, I've fixed them below.
Now, do you have a GCC copyright assignment or are under copyright
assignment of some company working on GCC (ARM?)?
2014-03-03 Lin Zuojian manjian2...@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.
--- gcc/asan.c.jj 2014-01-09 19:09:45.981593894 +0100
+++ gcc/asan.c 2014-03-04 08:55:22.239816211 +0100
@@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rt
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, rt
(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, rt
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;
--- gcc/cfgexpand.c.jj 2014-03-03 08:25:17.164537912 +0100
+++ gcc/cfgexpand.c 2014-03-04 08:57:53.860949703 +0100
@@ -1013,10 +1013,19 @@ expand_stack_vars (bool (*pred) (size_t)
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
{
@@ -1843,6 +1852,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,
Jakub
Re: [PATCH v7] PR middle-end/60281
On Tue, Mar 04, 2014 at 09:04:56AM +0100, Jakub Jelinek wrote: On Tue, Mar 04, 2014 at 11:11:45AM +0800, lin zuojian wrote: Without aligning the asan stack base,this base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * 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. There were still a couple of formatting issues, I've fixed them below. Now, do you have a GCC copyright assignment or are under copyright assignment of some company working on GCC (ARM?)? Thanks for the fixing!. I have none of them.I have read the website,but don't know what exactly I am expecting to do.
Re: [PATCH v7] PR middle-end/60281
On Tue, Mar 04, 2014 at 04:44:57PM +0800, lin zuojian wrote: On Tue, Mar 04, 2014 at 09:04:56AM +0100, Jakub Jelinek wrote: On Tue, Mar 04, 2014 at 11:11:45AM +0800, lin zuojian wrote: Without aligning the asan stack base,this base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * 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. There were still a couple of formatting issues, I've fixed them below. Now, do you have a GCC copyright assignment or are under copyright assignment of some company working on GCC (ARM?)? Thanks for the fixing!. I have none of them.I have read the website,but don't know what exactly I am expecting to do. Please follow http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future Jakub
