diamondburned/gotk4

ConstraintLayout.AddConstraintsFromDescription results in SIGSEGV

Matir opened this issue · 1 comments

Matir commented

I was trying to use a constraint-based layout, but doing so resulted in SIGSEGV when trying to load from a description. I've minimized the test case as best as I can here:

package main

import (
	"os"

	gio "github.com/diamondburned/gotk4/pkg/gio/v2"
	gtk "github.com/diamondburned/gotk4/pkg/gtk/v4"
)

func main() {
	app := gtk.NewApplication("com.example", gio.ApplicationFlagsNone)
	app.ConnectActivate(func() {
		foo := gtk.NewLabel("foo")
		bar := gtk.NewLabel("bar")
		box := gtk.NewBox(gtk.OrientationVertical, 0)
		box.Append(foo)
		box.Append(bar)
		layout := gtk.NewConstraintLayout()
		box.SetLayoutManager(layout)
		views := map[string]gtk.ConstraintTargetter{
			"foo": foo,
			"bar": bar,
		}
		constraints := []string{}
		layout.AddConstraintsFromDescription(constraints, 0, 0, views)
		win := gtk.NewApplicationWindow(app)
		win.SetTitle("ConstraintLayout test")
		win.SetChild(box)
		win.Show()
	})
	app.Run(os.Args)
}

Specifying a non-empty views is required for the crash. Here's the go backtrace:

go run .   
fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x7fd3fc35de59]

runtime stack:
runtime.throw({0x99dc17?, 0x0?})
	/usr/lib/go/src/runtime/panic.go:1047 +0x5d fp=0x7ffdcc5be4b0 sp=0x7ffdcc5be480 pc=0x58285d
runtime.sigpanic()
	/usr/lib/go/src/runtime/signal_unix.go:821 +0x3e9 fp=0x7ffdcc5be510 sp=0x7ffdcc5be4b0 pc=0x598a89

goroutine 1 [syscall, locked to thread]:
runtime.cgocall(0x7ee810, 0xc000115770)
	/usr/lib/go/src/runtime/cgocall.go:157 +0x5c fp=0xc000115748 sp=0xc000115710 pc=0x550a5c
github.com/diamondburned/gotk4/pkg/gtk/v4._Cfunc_g_hash_table_unref(0x242be40)
	_cgo_gotypes.go:5704 +0x45 fp=0xc000115770 sp=0xc000115748 pc=0x6e4fc5
github.com/diamondburned/gotk4/pkg/gtk/v4.(*ConstraintLayout).AddConstraintsFromDescription.func5.1()
	/home/david/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.4/gtk/v4/gtkconstraintlayout.go:403 +0x46 fp=0xc0001157a8 sp=0xc000115770 pc=0x70c5a6
runtime.deferreturn()
	/usr/lib/go/src/runtime/panic.go:476 +0x33 fp=0xc0001157e8 sp=0xc0001157a8 pc=0x5812b3
github.com/diamondburned/gotk4/pkg/gtk/v4.(*ConstraintLayout).AddConstraintsFromDescription(0xc000012120, {0xc000115a20?, 0x0, 0x0}, 0x0, 0x0, 0xc000115a58)
	/home/david/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.4/gtk/v4/gtkconstraintlayout.go:426 +0x595 fp=0xc0001159e8 sp=0xc0001157e8 pc=0x70c175
main.main.func1()
	/home/david/tmp/gtkerr/main.go:25 +0x1c7 fp=0xc000115ba8 sp=0xc0001159e8 pc=0x7b5e07
github.com/diamondburned/gotk4/pkg/gio/v2._gotk4_gio2_Application_ConnectActivate(0x551046?, 0xc0000061a0?)
	/home/david/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.4/gio/v2/gapplication.go:592 +0x74 fp=0xc000115bf0 sp=0xc000115ba8 pc=0x642894
_cgoexp_8ebff343743b__gotk4_gio2_Application_ConnectActivate(0x7ffdcc5be2e0?)
	_cgo_gotypes.go:29186 +0x25 fp=0xc000115c10 sp=0xc000115bf0 pc=0x693165
runtime.cgocallbackg1(0x693140, 0xc000115dc0?, 0x0)
	/usr/lib/go/src/runtime/cgocall.go:315 +0x2b1 fp=0xc000115cd8 sp=0xc000115c10 pc=0x550f51
runtime.cgocallbackg(0xc0000061a0?, 0x300000002?, 0xc0000061a0?)
	/usr/lib/go/src/runtime/cgocall.go:234 +0x109 fp=0xc000115d68 sp=0xc000115cd8 pc=0x550c09
runtime.cgocallbackg(0x693140, 0x7ffdcc5be670, 0x0)
	<autogenerated>:1 +0x2f fp=0xc000115d90 sp=0xc000115d68 pc=0x5b51ef
runtime.cgocallback(0x550a85, 0x7c2c60, 0xc000115e20)
	/usr/lib/go/src/runtime/asm_amd64.s:998 +0xb4 fp=0xc000115db8 sp=0xc000115d90 pc=0x5b2ab4
runtime.systemstack_switch()
	/usr/lib/go/src/runtime/asm_amd64.s:463 fp=0xc000115dc0 sp=0xc000115db8 pc=0x5b0ac0
runtime.cgocall(0x7c2c60, 0xc000115e20)
	/usr/lib/go/src/runtime/cgocall.go:167 +0x85 fp=0xc000115df8 sp=0xc000115dc0 pc=0x550a85
github.com/diamondburned/gotk4/pkg/gio/v2._Cfunc_g_application_run(0x21990e0, 0x1, 0x219bf70)
	_cgo_gotypes.go:5526 +0x4c fp=0xc000115e20 sp=0xc000115df8 pc=0x62d42c
github.com/diamondburned/gotk4/pkg/gio/v2.(*Application).Run.func3(0xc000165680?, 0x27?, 0x219bf70?)
	/home/david/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.4/gio/v2/gapplication.go:1619 +0x71 fp=0xc000115e68 sp=0xc000115e20 pc=0x643e51
github.com/diamondburned/gotk4/pkg/gio/v2.(*Application).Run(0xc0000120a8, {0xc000016050?, 0x1, 0x1})
	/home/david/go/pkg/mod/github.com/diamondburned/gotk4/pkg@v0.0.4/gio/v2/gapplication.go:1619 +0x1e5 fp=0xc000115f28 sp=0xc000115e68 pc=0x643d25
main.main()
	/home/david/tmp/gtkerr/main.go:31 +0xb4 fp=0xc000115f80 sp=0xc000115f28 pc=0x7b5c14
runtime.main()
	/usr/lib/go/src/runtime/proc.go:250 +0x207 fp=0xc000115fe0 sp=0xc000115f80 pc=0x585207
runtime.goexit()
	/usr/lib/go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc000115fe8 sp=0xc000115fe0 pc=0x5b2ce1

(Some goroutines elided.) Notice the failure in g_hash_table_unref. A backtrace from gdb isn't super revealing to me:

#0  0x00007ffff700ce59 in free () at /usr/lib/libc.so.6
#1  0x00007ffff7c45c13 in  () at /usr/lib/libglib-2.0.so.0
#2  0x00007ffff7c49ca4 in g_hash_table_unref () at /usr/lib/libglib-2.0.so.0
#3  0x00000000005b29a4 in runtime.asmcgocall () at /usr/lib/go/src/runtime/asm_amd64.s:848
#4  0x00007ffff59e38c0 in  ()
#5  0x00007fffffffc668 in  ()
#6  0x000000000055a7ce in runtime.persistentalloc.func1 () at /usr/lib/go/src/runtime/malloc.go:1393
#7  0x00000000005b0b29 in runtime.systemstack () at /usr/lib/go/src/runtime/asm_amd64.s:496
#8  0x00007fffffffcc58 in  ()
#9  0x0000000000df9290 in  ()
#10 0x0000000000d2ae40 in runtime.m0 ()
#11 0x00007fffffffc6e0 in  ()
#12 0x00000000005cf55d in crosscall2 () at /usr/lib/go/src/runtime/cgo/asm_amd64.s:30
#13 0x00000000007b76e6 in _gotk4_gio2_Application_ConnectActivate (arg0=0x1c4, arg1=0xc00013b738) at _cgo_export.c:1037
#14 0x00007ffff7d59210 in g_closure_invoke () at /usr/lib/libgobject-2.0.so.0
#15 0x00007ffff7d86eb8 in  () at /usr/lib/libgobject-2.0.so.0
#16 0x00007ffff7d76f85 in g_signal_emit_valist () at /usr/lib/libgobject-2.0.so.0
#17 0x00007ffff7d77214 in g_signal_emit () at /usr/lib/libgobject-2.0.so.0
#18 0x00007ffff7e9b389 in  () at /usr/lib/libgio-2.0.so.0
#19 0x00007ffff7ea66dc in g_application_run () at /usr/lib/libgio-2.0.so.0
#20 0x00000000007c2c7e in _cgo_8ebff343743b_Cfunc_g_application_run (v=0xc00013be20) at /tmp/go-build/cgo-gcc-prolog:453
#21 0x00000000005b29a4 in runtime.asmcgocall () at /usr/lib/go/src/runtime/asm_amd64.s:848

Looking at the generated source, I note that the g_hash_table has functions specified to free both keys and values (https://github.com/diamondburned/gotk4/blob/pkg/v0.0.4/pkg/gtk/v4/gtkconstraintlayout.go#L394). However, it's not clear to me that the value involved (a GObject for the target) is allocated on the C heap vs the go heap, or that it should be freed at any point in the process of adding constraints from a description.

Additionally, it looks like there's a risk of a double free because kdst is explicitly freed in a defer (https://github.com/diamondburned/gotk4/blob/pkg/v0.0.4/pkg/gtk/v4/gtkconstraintlayout.go#L399) but should also be freed when the hash table is deallocated.

Matir commented

Follow-up: this seems to be because the GLib.HashTable case in gocConvertNested claims to only support map[string]string but this is not map[string]string. The code that checks this case outputs a Debug statement, but does not return early, leading to the attempt to convert: https://github.com/diamondburned/gotk4/blame/pkg/v0.0.4/gir/girgen/types/typeconv/go-c.go#L406. The conversion code thinks it owns the objects involved due to the conversion for the string type operating that way, but that's not applicable in this case.