This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v5] PR middle-end/60281
- From: Jakub Jelinek <jakub at redhat dot com>
- To: lin zuojian <manjian2006 at gmail dot com>
- Cc: ramrad01 at arm dot com, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 3 Mar 2014 14:23:55 +0100
- Subject: Re: [PATCH v5] PR middle-end/60281
- Authentication-results: sourceware.org; auth=none
- References: <530C460E dot 5080104 at gmail dot com> <20140303085120 dot GB11890 at ubuntu> <20140303085859 dot GB22862 at tucnak dot redhat dot com> <20140303090946 dot GA10357 at ubuntu> <20140303131028 dot GG10357 at ubuntu>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Mar 03, 2014 at 09:10:28PM +0800, lin zuojian wrote:
> +2014-03-03 lin zuojian <manjian2006@gmail.com>
Capital letters at the beginning of name/surname please.
One empty line after the date/name/mail line.
> + Fix PR middle-end/60281
Remove "Fix ", just
PR middle-end/60281
> + * asan.c (asan_emit_stack_protection): Forcing the base to align to
Force instead of Forcing.
> + appropriate bits if STRICT_ALIGNMENT.And set shadow_mem align to
Two spaces after dot, remove And, just write . Set shadow_mem align ...
> + appropriate bits if STRICT_ALIGNMENT.
> + * cfgexpand.c (expand_stack_vars): set base_align appropriately when asan is
Capital S in Set. Too long line (at most 80 chars), so wrap after when?
> + on.
> + (expand_used_vars): Leaving a space in the stack frame for alignment if
> + STRICT_ALIGNMENT.
Leave.
> index 53992a8..64898cd 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -1017,8 +1017,16 @@ 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);
Wrong formatting. base should start 2 spaces after if, so something like:
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);
> +
> +
If you really want, just add one vertical space here, certainly not two.
> base = expand_binop (Pmode, add_optab, base,
> gen_int_mode (base_offset - base_align_bias, Pmode),
> NULL_RTX, 1, OPTAB_DIRECT);
> @@ -1097,6 +1105,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)));
set_mem_align should be indented by 4 spaces, missing space before opening (
after set_mem_align.
> @@ -1186,6 +1196,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)));
> +
Likewise.
> @@ -1013,10 +1013,18 @@ 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));
Wrong formatting, if should be aligned below base = data->asan_base,
base_align = two spaces more. Space in between MAX and (, no need for the
extra parens around the <<.
> }
> else
> + {
> offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
> - base_align = crtl->max_used_stack_slot_alignment;
> + base_align = crtl->max_used_stack_slot_alignment;
> + }
Again, wrong indentation.
> }
> else
> {
> @@ -1843,6 +1851,9 @@ 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 a space for alignment if STRICT_ALIGNMENT. */
> + if (STRICT_ALIGNMENT)
> + alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode) << ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
Likewise, too long line, which needs to be split, and extra space before ,
Jakub