"William J. Schmidt" <[email protected]> writes:
> I'm investigating another build failure for Fedora 17 (based on 4.7).
> The failing compile from the build log is as follows:
>
> /bin/sh ./libtool --tag=CC
> --mode=compile
> /builddir/build/BUILD/gcc-4.7.0-20120504/obj-ppc64-redhat-linux/./gcc/xgcc
> -B/builddir/build/BUILD/gcc-4.7.0-20120504/obj-ppc64-redhat-linux/./gcc/
> -B/usr/ppc64-redhat-linux/bin/ -B/usr/ppc64-redhat-linux/lib/ -isystem
> /usr/ppc64-redhat-linux/include -isystem /usr/ppc64-redhat-linux/sys-include
> -DHAVE_CONFIG_H -I. -I../../../libgo -I ../../../libgo/runtime
> -I../../../libgo/../libffi/include -I../libffi/include -pthread -fexceptions
> -fplan9-extensions -Wall -Wextra -Wwrite-strings -Wcast-qual -Werror
> -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I
> ../../../libgo/../libgcc -I ../../gcc/include -O2 -O3 -mtune=power7
> -mcpu=power7 -g -fsigned-char -MT proc.lo -MD -MP -MF .deps/proc.Tpo -c -o
> proc.lo `test -f 'runtime/proc.c' || echo '../../../libgo/'`runtime/proc.c
>
> The reported failure:
>
> ../../../libgo/runtime/proc.c: In function ‘__go_go’:
> ../../../libgo/runtime/proc.c:1323:8: error: variable ‘sp’ might be
> clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> ../../../libgo/runtime/proc.c:1324:9: error: variable ‘spsize’ might be
> clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> cc1: all warnings being treated as errors
>
> The declarations in question are:
>
> byte *sp;
> size_t spsize;
> G * volatile newg; // volatile to avoid longjmp warning
>
> What's interesting here is that I don't see a problem when compiling for
> a 32-bit target. However, it's also apparent that making "newg"
> volatile was deemed sufficient up till now, so I am wondering if there
> is something about our build options/environment that could cause the
> problem.
>
> I can work around the problem with the following patch, but would
> appreciate any insights into whether this should be necessary, and if
> not, what we're doing "wrong."
I don't know why you are seeing this when I am not. However, the
warning is essentially correct. It's not useful, because as it happens
nothing will ever call setcontext in such a way that the getcontext call
will return a second time, but the compiler can't know that. And if it
did happen, the warning would be valid. So I committed this variant of
your patch to prevent the warning. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline and 4.7 branch.
Ian
diff -r efd2462cb004 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c Mon May 14 14:57:59 2012 -0700
+++ b/libgo/runtime/proc.c Tue May 15 11:54:04 2012 -0700
@@ -1322,7 +1322,7 @@
{
byte *sp;
size_t spsize;
- G * volatile newg; // volatile to avoid longjmp warning
+ G *newg;
schedlock();
@@ -1363,19 +1363,26 @@
if(sp == nil)
runtime_throw("nil g->stack0");
- getcontext(&newg->context);
- newg->context.uc_stack.ss_sp = sp;
+ {
+ // Avoid warnings about variables clobbered by
+ // longjmp.
+ byte * volatile vsp = sp;
+ size_t volatile vspsize = spsize;
+ G * volatile vnewg = newg;
+
+ getcontext(&vnewg->context);
+ vnewg->context.uc_stack.ss_sp = vsp;
#ifdef MAKECONTEXT_STACK_TOP
- newg->context.uc_stack.ss_sp += spsize;
+ vnewg->context.uc_stack.ss_sp += vspsize;
#endif
- newg->context.uc_stack.ss_size = spsize;
- makecontext(&newg->context, kickoff, 0);
+ vnewg->context.uc_stack.ss_size = vspsize;
+ makecontext(&vnewg->context, kickoff, 0);
- newprocreadylocked(newg);
- schedunlock();
+ newprocreadylocked(vnewg);
+ schedunlock();
- return newg;
-//printf(" goid=%d\n", newg->goid);
+ return vnewg;
+ }
}
// Put on gfree list. Sched must be locked.
