diff mbox

Build problem with libgo runtime

Message ID mcr8vgt46ey.fsf@dhcp-172-18-216-180.mtv.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor May 15, 2012, 6:59 p.m. UTC 
"William J. Schmidt" <wschmidt@linux.vnet.ibm.com> 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 mbox

Patch

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.