diff mbox

libgo patch committed: Change some stack fields to uintptr

Message ID yddy3sjevx2.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth June 22, 2017, 6:09 p.m. UTC 
Hi Ian,

> Because of how gccgo implements cgo calls, the code in dropm may not
> have any write barriers.  As a step toward implementing that, change
> the gcstack, gcnextsegment, and gcnextsp fields of the g struct to
> uintptr, so that assignments to them do not require write barriers.
> The gcinitialsp field remains unsafe.Pointer, as on 32-bit systems
> that do not support split stack it points to a heap allocated space
> used for the goroutine stack.
>
> The test for this is runtime tests like TestCgoCallbackGC, which are
> not run today but will be run with a future gotools patch.
>
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

this patch broke bootstrap on non-split-stack targets like Solaris:

/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function âdoentersyscallblockâ:
/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:681:15: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
   g->gcnextsp = (byte *) &v;
               ^

The patch below allowed sparc-sun-solaris2.12 and i386-pc-solaris2.12
builds to finish.

	Rainer

Comments

Ian Lance Taylor June 22, 2017, 7:52 p.m. UTC | #1 
On Thu, Jun 22, 2017 at 11:09 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
>
>> Because of how gccgo implements cgo calls, the code in dropm may not
>> have any write barriers.  As a step toward implementing that, change
>> the gcstack, gcnextsegment, and gcnextsp fields of the g struct to
>> uintptr, so that assignments to them do not require write barriers.
>> The gcinitialsp field remains unsafe.Pointer, as on 32-bit systems
>> that do not support split stack it points to a heap allocated space
>> used for the goroutine stack.
>>
>> The test for this is runtime tests like TestCgoCallbackGC, which are
>> not run today but will be run with a future gotools patch.
>>
>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>> to mainline.
>
> this patch broke bootstrap on non-split-stack targets like Solaris:
>
> /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function âdoentersyscallblockâ:
> /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:681:15: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
>    g->gcnextsp = (byte *) &v;
>                ^
>
> The patch below allowed sparc-sun-solaris2.12 and i386-pc-solaris2.12
> builds to finish.

Sorry, I thought I caught all of those.  Patch committed.  Thanks.

Ian
diff mbox

Patch

diff --git a/libgo/runtime/proc.c b/libgo/runtime/proc.c
--- a/libgo/runtime/proc.c
+++ b/libgo/runtime/proc.c
@@ -678,7 +678,7 @@  doentersyscallblock(uintptr pc, uintptr 
 	{
 		void *v;
 
-		g->gcnextsp = (byte *) &v;
+		g->gcnextsp = (uintptr)(&v);
 	}
 #endif