gl: gl.FenceSync returns invalid unsafe.Pointer, causing crash

gl.FenceSync returns the created fence as an unsafe.Pointer. However, values returned by glFenceSync aren’t necessarily valid pointers to memory owned by the driver.

On my system (nvidia 378.13 on Linux), for example, glFenceSync returns increasing numbers, starting at 1. The issue with that is that the Go runtime expects pointers in unsafe.Pointer to be valid, either pointing to memory owned by the Go memory allocator, or valid, non-Go memory. Pointers such as “1” are neither, which causes runtime.writebarrierptr to abort the program with a “bad pointer in write barrier” error. Similarly, the driver might return a value that looks like a valid pointer to Go memory, preventing said Go memory from being garbage collected as long as the unsafe.Pointer is alive.

To simulate the issue, compile https://play.golang.org/p/614oIx_Y1D and run it with GOGC=0 (to force frequent GCs, triggering the problem faster.). You should see the following:

$ GOGC=0 ./foo
runtime: writebarrierptr *0x4fa230 = 0x1
fatal error: bad pointer in write barrier

runtime stack:
runtime.throw(0x4a81ca, 0x1c)
	/usr/lib/go/src/runtime/panic.go:596 +0x95
runtime.writebarrierptr.func1()
	/usr/lib/go/src/runtime/mbarrier.go:208 +0xbd
runtime.systemstack(0x4fab00)
	/usr/lib/go/src/runtime/asm_amd64.s:327 +0x79
runtime.mstart()
	/usr/lib/go/src/runtime/proc.go:1132

goroutine 1 [running]:
runtime.systemstack_switch()
	/usr/lib/go/src/runtime/asm_amd64.s:281 fp=0xc420048f00 sp=0xc420048ef8
runtime.writebarrierptr(0x4fa230, 0x1)
	/usr/lib/go/src/runtime/mbarrier.go:209 +0x96 fp=0xc420048f38 sp=0xc420048f00
main.main()
	/tmp/foo.go:15 +0x9b fp=0xc420048f88 sp=0xc420048f38
runtime.main()
	/usr/lib/go/src/runtime/proc.go:185 +0x20a fp=0xc420048fe0 sp=0xc420048f88
runtime.goexit()
	/usr/lib/go/src/runtime/asm_amd64.s:2197 +0x1 fp=0xc420048fe8 sp=0xc420048fe0

I would recommend returning a uintptr instead (and, similarly, accepting a uintptr in gl.ClientWaitSync and gl.WaitSync).

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 26 (16 by maintainers)

Commits related to this issue

all: Represent GLsync as uintptr, not unsafe.Pointer. Regenerate all bindings after generator change in go-gl/glow#79. Done with latest version of glow: go generate -tags=gen github.com/go-gl/gl ... — committed to go-gl/gl by dmitshur 7 years ago

Most upvoted comments

Alas. Well, because of https://github.com/go-gl/gl/issues/80, I’m expecting a more broadly breaking API change will be necessary, so hopefully we can break everyone only once.

errcw on Feb 18, 2018

Like I said, it’s working 😃

dominikh on Oct 21, 2018

I’ve finally gotten around to testing the change. It compiles, doesn’t cause runtime panics, and I’ve successfully used a fence.

dominikh on Feb 16, 2018

Right, it should be straightforward to update type.go in glow to change the Go type of GLsync, and I agree that’s the right path forward. It’s unfortunate it’s not a backwards-compatible change, but given that it’s required, I don’t think there is a better way.

errcw on May 9, 2017

It’s not quite the same as #31, but does fall into the category of not playing loose with pointer types.

Regarding documentation: https://www.khronos.org/opengl/wiki/Sync_Object and the Go spec would be relevant here. The former specifies that GLsync is an opaque type typedef struct __GLsync *GLsync, ergo a pointer. And uintptr is defined as an unsigned integer large enough to store the uninterpreted bits of a pointer value.

Good point re gl.IsSync – I haven’t looked up the whole list of GLsync-related functions.

Do you want to send a PR (to go-gl/glow, the generator that generates these bindings)?

I can do that if my proposed solution is accepted.

dominikh on May 9, 2017