Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

imguiWrapperTypes.h should use "uintptr_t" instead of "void*" #98

Closed
mdempsky opened this issue Apr 30, 2020 · 8 comments
Closed

imguiWrapperTypes.h should use "uintptr_t" instead of "void*" #98

mdempsky opened this issue Apr 30, 2020 · 8 comments

Comments

@mdempsky
Copy link

In Go 1.14, the Go compiler added a "checkptr" feature that detects misuse of unsafe.Pointer. It's triggering within imgui-go because of conversions like here:

return C.IggTextureID(id)

The problem is IggTextureID is declared in imguiWrapperTypes.h as typedef void *IggTextureID. This makes cgo use unsafe.Pointer when representing this type in Go.

If you instead #include <stdint.h> and use typedef uintptr_t IggTextureID, the error should go away.

dertseha added a commit that referenced this issue May 17, 2020
As this value is not a proper pointer, arbitrary values can be saved in there. It still must be big enough to hold pointers, as per underlying pointer type in ImGui.
@dertseha
Copy link
Member

Thank you for raising this.
I have changed the type for IggTextureID to uintptr_t . For the others this should not be necessary, as they do hold actual pointer to the types.

I'm still trying to reproduce the warning on my side, can you confirm in parallel that this fixes your issue?

@JetSetIlly
Copy link
Contributor

This fixes the immediate issue I was having. However, a similar error remains. Building imgui-go-examples/cmd/example_sdl_opengl3 with go run -race -tags 'sdl' . (after bumping go.mod to latest version):

fatal error: checkptr: unsafe pointer arithmetic

goroutine 1 [running, locked to thread]:
runtime.throw(0x6fd0fd, 0x23)
/home/steve/Go/go-current/src/runtime/panic.go:1114 +0x72 fp=0xc00006da68 sp=0xc00006da38 pc=0x4b8f22
runtime.checkptrArithmetic(0x8, 0x0, 0x0, 0x0)
/home/steve/Go/go-current/src/runtime/checkptr.go:26 +0xce fp=0xc00006da98 sp=0xc00006da68 pc=0x48ebae
github.com/inkyblackness/imgui-go-examples/internal/renderers.(*OpenGL3).Render(0xc000100050, 0x4434000044a00000, 0x4434000044a00000, 0x268b380)
/home/steve/Go/steve/imgui-go-examples/internal/renderers/OpenGL3.go:144 +0x112d fp=0xc00006dce0 sp=0xc00006da98 pc=0x5bafcd
github.com/inkyblackness/imgui-go-examples/internal/demo.Run(0x721e00, 0xc000114210, 0x7206e0, 0xc000100050)
/home/steve/Go/steve/imgui-go-examples/internal/demo/Run.go:117 +0x2ba fp=0xc00006ddf8 sp=0xc00006dce0 pc=0x57da8a
main.main()
/home/steve/Go/steve/imgui-go-examples/cmd/example_sdl_opengl3/main.go:35 +0x2d8 fp=0xc00006df88 sp=0xc00006ddf8 pc=0x5bdcb8
runtime.main()
/home/steve/Go/go-current/src/runtime/proc.go:203 +0x212 fp=0xc00006dfe0 sp=0xc00006df88 pc=0x4bb5a2
runtime.goexit()
/home/steve/Go/go-current/src/runtime/asm_amd64.s:1373 +0x1 fp=0xc00006dfe8 sp=0xc00006dfe0 pc=0x4e7681
exit status 2

@dertseha
Copy link
Member

OK, thank you for the reproduction steps. I'll have a look.

@mdempsky
Copy link
Author

go vet should be able to report suspicious uintptr-to-unsafe.Pointer conversions as well.

@dertseha
Copy link
Member

Welp, now we're at the next library :)
Because this now is an issue of go-gl/gl .

Since that is used via the examples, this issue here can be closed, and the new problem can be tracked via inkyblackness/imgui-go-examples#3 .

Thank you all for raising this - it helps making APIs cleaner.

@JetSetIlly
Copy link
Contributor

Yup. I thought so too but I don't enough about this end of the language to be helpful. I see someone has raised the issue in go-gl already.

@dertseha
Copy link
Member

In case anyone is still watching this, imgui-go-examples has finally been modified to use checkptr safe functions from the newest go-gl/gl library. It can now be run with the detector active.

@JetSetIlly
Copy link
Contributor

Excellent

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants