Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkptr failure #7

Closed
JetSetIlly opened this issue Mar 29, 2020 · 5 comments
Closed

Checkptr failure #7

JetSetIlly opened this issue Mar 29, 2020 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@JetSetIlly
Copy link
Owner

JetSetIlly commented Mar 29, 2020

As of version 1.14 of Go the race detector adds checkptr instrumention. This compilation option ensures that program's follow Go's unsafe.Pointer safety rules. https://golang.org/doc/go1.14#compiler

Currently, Gopher2600 fails the checkptr test because of how fonts are setup for imgui-go. To wit, the following triggers the checkptr error:

// Store our identifier
rnd.imguiIO.Fonts().SetTextureID(imgui.TextureID(rnd.fontTexture))

fontTexture has previously been generated and bound and has the value 0. imgui.TextureID casts that value to a uintptr and thus triggers the error (pointer value too low).

It is possible to turn off checkptr with a build flag (the Makefile includes a race target that does this):

go run -race -gcflags=all=-d=checkptr=0 gopher2600.go debug

This is useful because it has allowed gopher2600 to be cleaned of race errors but the checkptr error remains.

In a nutshell, I'm not sure how to fix it. As far as I can tell, the fontTexture value is not really a pointer but casting it to uintptr raises the suspicions of the checkPtrcode. I also cannot see how to alter imgui-go in such a way to avoid using uintptr.

@JetSetIlly JetSetIlly added the help wanted Extra attention is needed label Mar 29, 2020
@JetSetIlly JetSetIlly pinned this issue Mar 30, 2020
@mdempsky
Copy link

Hi, I added the checkptr feature to the Go compiler. Are you still having this problem? If so, can you share the checkptr error message and stack trace that are printed?

@JetSetIlly
Copy link
Owner Author

Hi Matthew, thanks for looking at this. Complete stack trace below:

fatal error: checkptr: unsafe pointer arithmetic

goroutine 1 [running, locked to thread]:
runtime.throw(0xb7c05c, 0x23)
/home/steve/Go/go-current/src/runtime/panic.go:1114 +0x72 fp=0xc000185720 sp=0xc0001856f0 pc=0x484842
runtime.checkptrArithmetic(0x1, 0x0, 0x0, 0x0)
/home/steve/Go/go-current/src/runtime/checkptr.go:26 +0xce fp=0xc000185750 sp=0xc000185720 pc=0x45723e
github.com/inkyblackness/imgui-go/v2.TextureID.handle(...)
/home/steve/Go/go/pkg/mod/github.com/!jet!set!illy/imgui-go/v2@v2.2.1-0.20200317095507-51a6e45b93a9/TextureID.go:24
github.com/inkyblackness/imgui-go/v2.FontAtlas.SetTextureID.func1(0x1d6e580, 0x1)
/home/steve/Go/go/pkg/mod/github.com/!jet!set!illy/imgui-go/v2@v2.2.1-0.20200317095507-51a6e45b93a9/FontAtlas.go:163 +0x78 fp=0xc000185790 sp=0xc000185750 pc=0x908768
github.com/inkyblackness/imgui-go/v2.FontAtlas.SetTextureID(0x1d6e580, 0x1)
/home/steve/Go/go/pkg/mod/github.com/!jet!set!illy/imgui-go/v2@v2.2.1-0.20200317095507-51a6e45b93a9/FontAtlas.go:163 +0x43 fp=0xc0001857b0 sp=0xc000185790 pc=0x901f23
github.com/jetsetilly/gopher2600/gui/sdlimgui.(*glsl).setup(0xc0000b4000)
/home/steve/gopher2600/gui/sdlimgui/glsl.go:439 +0x1b75 fp=0xc000185b28 sp=0xc0001857b0 pc=0x955315
github.com/jetsetilly/gopher2600/gui/sdlimgui.newGlsl(0x1d6a818, 0xc0000a4000, 0x0, 0x0, 0xa57700)
/home/steve/gopher2600/gui/sdlimgui/glsl.go:92 +0x1d4 fp=0xc000185bb0 sp=0xc000185b28 pc=0x950364
github.com/jetsetilly/gopher2600/gui/sdlimgui.NewSdlImgui(0xc47c00, 0xc0001543c0, 0xc000185ed8, 0x4, 0x4)
/home/steve/gopher2600/gui/sdlimgui/sdlimgui.go:107 +0x39a fp=0xc000185d48 sp=0xc000185bb0 pc=0x95c51a
main.debug.func1(0xc000185ed8, 0xc000185df8, 0x4, 0x1)
/home/steve/gopher2600/gopher2600.go:340 +0x4f fp=0xc000185d98 sp=0xc000185d48 pc=0x9beccf
main.main()
/home/steve/gopher2600/gopher2600.go:136 +0x505 fp=0xc000185f88 sp=0xc000185d98 pc=0x9b73c5
runtime.main()
/home/steve/Go/go-current/src/runtime/proc.go:203 +0x212 fp=0xc000185fe0 sp=0xc000185f88 pc=0x486ec2
runtime.goexit()
/home/steve/Go/go-current/src/runtime/asm_amd64.s:1373 +0x1 fp=0xc000185fe8 sp=0xc000185fe0 pc=0x4b69f1

goroutine 35 [syscall]:
os/signal.signal_recv(0x4b69f1)
/home/steve/Go/go-current/src/runtime/sigqueue.go:147 +0x9c
os/signal.loop()
/home/steve/Go/go-current/src/os/signal/signal_unix.go:23 +0x30
created by os/signal.Notify.func1
/home/steve/Go/go-current/src/os/signal/signal.go:127 +0x7c

goroutine 20 [select]:
main.debug(0xc00018e200, 0xc0001204c0, 0x0, 0x0)
/home/steve/gopher2600/gopher2600.go:352 +0x9a4
main.launch(0xc0001204c0)
/home/steve/gopher2600/gopher2600.go:201 +0x5c4
created by main.main
/home/steve/gopher2600/gopher2600.go:111 +0x25f

goroutine 37 [select]:
github.com/jetsetilly/gopher2600/television.NewTelevision.func1(0xc0001543c0)
/home/steve/gopher2600/television/television.go:177 +0x2ac
created by github.com/jetsetilly/gopher2600/television.NewTelevision
/home/steve/gopher2600/television/television.go:170 +0x36e
exit status 2

@mdempsky
Copy link

Thanks. It looks like this is an issue with imgui-go.

The problem isn't that you're converting a value to uintptr. It's that imgui-go's TextureID.handle method is converting a uintptr to C.IggTextureID, where C.IggTextureID is defined as typedef void *IggTextureID in C.

Probably imgui-go should be updated to use typedef uintptr_t IggTextureId instead.

@mdempsky
Copy link

Filed inkyblackness/imgui-go#98

@JetSetIlly
Copy link
Owner Author

Fixed by changes described here inkyblackness/imgui-go#98

@JetSetIlly JetSetIlly unpinned this issue Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants