On Thu, Feb 9, 2012 at 12:54 AM, Carrot Wei <car...@google.com> wrote: > Hi Richard and Jakub > > Since 4.6 contains the same bug, I would like to back port it to 4.6 > branch. Could you approve it for 4.6? > > Jing and Doug > > Could you approve it for google/gcc-4_6-mobile branch? >
OK for google/gcc-4_6-mobile and gcc-4_6_2-mobile Jing > thanks > Carrot > > On Mon, Feb 6, 2012 at 9:14 PM, Richard Guenther > <richard.guent...@gmail.com> wrote: >> On Mon, Feb 6, 2012 at 2:01 PM, Jakub Jelinek <ja...@redhat.com> wrote: >>> Hi! >>> >>> The attached testcase is miscompiled on arm*, by doing a sibcall when setup >>> of one argument overwrites incoming arguments used to setup parameters in >>> later insns. >>> The reason why >>> mem_overlaps_already_clobbered_arg_p/check_sibcall_argument_overlap >>> fails to detect is that the caller has non-zero >>> crtl->args.pretend_args_size, and in that case the base: >>> /* The argument block when performing a sibling call is the >>> incoming argument block. */ >>> if (pass == 0) >>> { >>> argblock = crtl->args.internal_arg_pointer; >>> argblock >>> #ifdef STACK_GROWS_DOWNWARD >>> = plus_constant (argblock, crtl->args.pretend_args_size); >>> #else >>> = plus_constant (argblock, -crtl->args.pretend_args_size); >>> #endif >>> stored_args_map = sbitmap_alloc (args_size.constant); >>> sbitmap_zero (stored_args_map); >>> } >>> apparently isn't virtual-incoming-rtx, but that plus pretend_args_size >>> (8 in this case). When we store bits into stored_args_map sbitmap, >>> we use arg->locate.slot_offset.constant based values (or something different >>> for ARGS_GROW_DOWNWARD, but when mem_overlaps_already_clobbered_arg_p is >>> testing those bits, it uses just virtual-incoming-rtx offsets (or something >>> different for ARGS_GROW_DOWNWARD). This patch fixes it by adjusting the >>> virtual-incoming-rtx relative offset to be actually argblock relative >>> offset. >>> >>> Bootstrapped/regtested on x86_64-linux and i686-linux and tested on the >>> testcase on arm cross. Ok for trunk? >> >> Ok. >> >> Thanks, >> Richard. >> >>> 2012-02-06 Jakub Jelinek <ja...@redhat.com> >>> >>> PR target/52129 >>> * calls.c (mem_overlaps_already_clobbered_arg_p): If val is >>> CONST_INT_P, subtract resp. add crtl->args.pretend_args_size to it. >>> >>> * gcc.c-torture/execute/pr52129.c: New test. >>> >>> --- gcc/calls.c.jj 2012-02-01 14:44:27.000000000 +0100 >>> +++ gcc/calls.c 2012-02-06 10:19:12.112132905 +0100 >>> @@ -1808,6 +1808,11 @@ mem_overlaps_already_clobbered_arg_p (rt >>> return true; >>> else >>> i = INTVAL (val); >>> +#ifdef STACK_GROWS_DOWNWARD >>> + i -= crtl->args.pretend_args_size; >>> +#else >>> + i += crtl->args.pretend_args_size; >>> +#endif >>> >>> #ifdef ARGS_GROW_DOWNWARD >>> i = -i - size; >>> --- gcc/testsuite/gcc.c-torture/execute/pr52129.c.jj 2012-02-06 >>> 10:27:50.988876791 +0100 >>> +++ gcc/testsuite/gcc.c-torture/execute/pr52129.c 2012-02-06 >>> 10:25:26.000000000 +0100 >>> @@ -0,0 +1,28 @@ >>> +/* PR target/52129 */ >>> + >>> +extern void abort (void); >>> +struct S { void *p; unsigned int q; }; >>> +struct T { char a[64]; char b[64]; } t; >>> + >>> +__attribute__((noinline, noclone)) int >>> +foo (void *x, struct S s, void *y, void *z) >>> +{ >>> + if (x != &t.a[2] || s.p != &t.b[5] || s.q != 27 || y != &t.a[17] || z != >>> &t.b[17]) >>> + abort (); >>> + return 29; >>> +} >>> + >>> +__attribute__((noinline, noclone)) int >>> +bar (void *x, void *y, void *z, struct S s, int t, struct T *u) >>> +{ >>> + return foo (x, s, &u->a[t], &u->b[t]); >>> +} >>> + >>> +int >>> +main () >>> +{ >>> + struct S s = { &t.b[5], 27 }; >>> + if (bar (&t.a[2], (void *) 0, (void *) 0, s, 17, &t) != 29) >>> + abort (); >>> + return 0; >>> +} >>> >>> Jakub