gl: PtrOffset causes "cgo argument has Go pointer to Go pointer" runtime error

EDIT: using GODEBUG=cgocheck=0 can disable these checks but seems like kind of a hacky-workaround, especially if you need runtime pointer checks for other packages.

It seems that calling functions like gl.DrawElements using gl.PtrOffset to convert a gl buffer offset causes the Go runtime to intercept those offsets as if they were pointers into Go memory. This is just a toy example; I’ve run into this problem using large index/vertex buffers in other projects. Trying to index elements in the millions seems to semi-randomly throw errors.

Drilling down a bit:

// PtrOffset takes a pointer offset and returns a GL-compatible pointer.
// Useful for functions such as glVertexAttribPointer that take pointer
// parameters indicating an offset rather than an absolute memory address.
func PtrOffset(offset int) unsafe.Pointer {
	return unsafe.Pointer(uintptr(offset))
}

I’m already out of my depth here but I’m wondering why offsets have to be sent to OpenGL via CGO as unsafe.Pointer? Is there some obvious way around this error I’m missing?

Program to reproduce:

package main

import (
	"log"

	"github.com/go-gl/gl/v2.1/gl"
	"github.com/go-gl/glfw/v3.2/glfw"
)

// var pointers = make([]*int, 40*(1<<20))
var buffer = make([]byte, 48*(1<<20))

func main() {
	err := glfw.Init()

	if err != nil {
		log.Fatal(err)
	}

	window, err := glfw.CreateWindow(1024, 768, "Test", nil, nil)

	if err != nil {
		log.Fatal(err)
	}

	//for i := range pointers {
	//	pointers[i] = new(int)
	//}

	window.MakeContextCurrent()

	if err := gl.Init(); err != nil {
		log.Fatalln(err)
	}
	var vertBuf, indexBuf uint32

	gl.CreateBuffers(1, &vertBuf)
	gl.CreateBuffers(1, &indexBuf)

	gl.BindBuffer(gl.ARRAY_BUFFER, vertBuf)
	gl.BindBuffer(gl.ELEMENT_ARRAY_BUFFER, indexBuf)

	gl.BufferData(gl.ARRAY_BUFFER, len(buffer), gl.Ptr(buffer), gl.DYNAMIC_DRAW)
	gl.BufferData(gl.ELEMENT_ARRAY_BUFFER, len(buffer), gl.Ptr(buffer), gl.DYNAMIC_DRAW)

	// const NumDraws = 1000
	var NumDraws = len(buffer) / 4

	for i := 0; i < NumDraws; i++ {
		//offset := rand.Int() % 12000000 //+ 12000000
		offset := i

		//fmt.Printf("offset = %d", offset)

		gl.DrawElements(gl.TRIANGLES, 500000, gl.UNSIGNED_INT, gl.PtrOffset(offset))

		gl.DrawElementsInstancedARB(gl.TRIANGLES, 500000, gl.UNSIGNED_INT, gl.PtrOffset(offset), 1)
	}

	//for _, p := range pointers {
	//	*p++
	//}

	window.Destroy()
	glfw.Terminate()
}

About this issue

Original URL
State: open
Created 7 years ago
Comments: 25 (16 by maintainers)

Commits related to this issue

Generate with neclepsio/glow The difference from [go-gl](https://github.com/go-gl/gl) are: - WithOffset variants for some functions, so you don't have to pass pointers insteas of offsets (closes go-g... — committed to neclepsio/gl by neclepsio 4 years ago

Most upvoted comments

PRs for both glow and gl are up.

As noted in the PR and other comments, this is a start for the necessary “override” functions. My proposal is to still close this issue here when the PRs are merged and accept corresponding issues/PRs on demand for further functions. I doubt that this task will ever be complete, so there’s no point in keeping this issue open forever.

dertseha on Apr 25, 2021

In the meantime it gets fixed, you can use my fork.

neclepsio on Jun 19, 2020

Hello there! I’m assuming that this is the central discussion thread for all the related issues popping up left and right.

The pull request go-gl/glow#107 is my proposal to add overload capabilities. This way glow could be taught to add overloads with different signatures.

To continue the discussion, the following is unclear to me:

Is the proposed way acceptable?
Should there be a restrictive naming schema? (Everything with suffix WithOffset, or is a VertexAttribOffset also allowed?)
How many functions are affected? (Different view of this question: Is it OK to extend these overloads in smaller batches, provided by community?)
Are there cases not covered by this schema?

I was able to do a minimal test with the three functions I used as a sample - though it was tedious;

my generated code would want KHR include files, which I don’t have (I also noticed several changes in a reference build to what is currently checked in)
the go-gl/gl does not have modules
- which would be required for relative module usages during tests),
- and with module files, glow would not work / create overrides…

dertseha on May 23, 2020

WithOffset methods would work for me. Thanks.

jeff-emanuel on Feb 22, 2018

Aha, MultiDrawElements is additionally problematic, at least in the case where the indices parameter might contain pointers to index data rather than offsets in a bound buffer. When passing Go memory to C, cgo requires that the Go memory contain no pointers to other Go memory (AIUI Go wants to avoid transitive pinning). The implication is that only C-allocated memory will work for this use case, which is a definite ease-of-use hiccup.

My takeaway is that simply assuming we can transform the API to use uintptr is infeasible, because folks are actively using these pointer-or-offset parameters to pass pointers, so we should instead generate additional WithOffset methods that swap unsafe.Pointer for uintptr. Additionally, we should clarify in the README how folks should handle pointer-to-pointer use cases.

errcw on Feb 22, 2018

As I understand it, the TL;DR is that Go does not allow non-pointer values in pointer types such as unsafe.Pointer. A small offset is very unlikely to be a valid pointer, hence the crash. The challenge is that the bindings are auto-generated based on the OpenGL XML specs, and these offsets are pointer parameters in the specs, hence are encoded as Go pointer types. Instead we should probably use uintptr as the Go type, which should in theory sidestep the issue, but that’d be a huge breaking change to the bindings. +@shurcooL to comment once he’s back.

errcw on Jun 9, 2017