On 1/3/19 11:44 PM, Jakub Jelinek wrote:
> Hi!
>
> The prev_offset we push into the vector is supposed to be the end of
> the red zone for the variable, it needs to be ASAN_MIN_RED_ZONE_SIZE
> aligned, but by aligning it more we can end up with red zone with negative
> size. E.g. on the testcase, we have initial frame_offset -8 (because of
> -fstack-protector guard), we align it to -32 first (required that the
> start and end of the var block is 32 byte aligned (4 byte aligned in shadow
> memory)). The first variable needs 64 byte alignment, but is just 4 bytes,
> so the next after
> offset = alloc_stack_frame_space (size, alignment);
> we end up with frame_offset (== offset) -64 and the start of red zone is
> after that 4 byte variable, i.e. at -60. But the align_base tweak from
> -32 with 64 byte alignment gives -64.
Thank you Jakub for the patch.
I was also considering adding a sanity check that will ensure
the offsets in data->asan_vec are always in increasing order.
What do you think about it?
>
> Now, the stack vars are sorted first by large vs. small alignment (not
> relevant to x86), then by size and only afterwards by alignment, so similar
> problems could appear if we have some large variable followed by > 32 byte
> aligned very small variable.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
I can add here that asan.exp tests work both on ppc64le-linux-gnu and
ppc64-linux-gnu.
I'm going to trigger asan-bootstrap on one of them.
Martin
>
> 2019-01-03 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/88619
> * cfgexpand.c (expand_stack_vars): Only align prev_offset to
> ASAN_MIN_RED_ZONE_SIZE, not to maximum of that and alignb.
>
> * c-c++-common/asan/pr88619.c: New test.
>
> --- gcc/cfgexpand.c.jj 2019-01-01 12:37:17.328972145 +0100
> +++ gcc/cfgexpand.c 2019-01-03 14:11:16.209366858 +0100
> @@ -1130,7 +1130,7 @@ expand_stack_vars (bool (*pred) (size_t)
> prev_offset = frame_offset.to_constant ();
> }
> prev_offset = align_base (prev_offset,
> - MAX (alignb, ASAN_MIN_RED_ZONE_SIZE),
> + ASAN_MIN_RED_ZONE_SIZE,
> !FRAME_GROWS_DOWNWARD);
> tree repr_decl = NULL_TREE;
> unsigned HOST_WIDE_INT size
> --- gcc/testsuite/c-c++-common/asan/pr88619.c.jj 2019-01-03
> 14:16:00.779659964 +0100
> +++ gcc/testsuite/c-c++-common/asan/pr88619.c 2019-01-03 14:15:40.568994254
> +0100
> @@ -0,0 +1,14 @@
> +/* PR sanitizer/88619 */
> +/* { dg-do compile { target fstack_protector } } */
> +/* { dg-options "-fstack-protector-strong -fsanitize=address" } */
> +
> +typedef int A __attribute__((aligned (64)));
> +
> +int
> +main ()
> +{
> + A b;
> + int *p = &b;
> + *(p - 1) = 123;
> + __builtin_alloca (b);
> +}
>
> Jakub
>
