genbindings: fix use-after-free in virtual-override callbacks returning heap CABI types

[5k3105]

In plain English

If you subclass a Qt virtual method in Go and that method returns a heap type
(a string, []string, QList<T>, or a map), the binding frees the memory a
moment too early — so C++ reads memory that's already been freed. The most
visible symptom: a custom QAbstractItemModel in a QTreeView with
InternalMove drag-drop crashes the instant you start a drag
(std::bad_alloc). A QStandardItemModel-backed view is fine, because its drag
path is pure C++ with no Go callback.

The cause is a one-line lifetime mistake in the code generator: when a Go
callback hands its return value back to C++, the generated Go code adds a
defer C.free(...). For a normal argument that's correct (free it after the
C call). For a return value it's wrong — the defer runs the moment the Go
callback returns, which is before C++ has copied the data out. C++ then reads
freed memory.

The fix moves the free from the wrong place to the right place. Two small,
coordinated changes:

1. Stop freeing too early (Go side). Don't emit the defer C.free(...) on
   the return value, so the memory survives until C++ has it.
2. Free at the right time (C++ side). In the generated C++ trampoline, free
   that memory after it's been copied into the real Qt type.

Change 1 alone stops the crash but would leak the memory; change 2 reclaims it.
Together: no crash, no leak. This is systemic — it covers every overridden
virtual that returns a heap type, not just mimeTypes (which is simply the one
the drag-drop path happens to call first).

---

Details

Reproduction

A custom QAbstractItemModel in a QTreeView with InternalMove drag-drop
crashes with std::bad_alloc / SIGABRT the moment a drag starts. Tested with
miqt v0.14.0 (qt6), Qt 6.11.1, Go 1.26, linux/amd64.

package main

import (
	"os"
	"strconv"

	qt "github.com/mappu/miqt/qt6"
)

const reproMime = "application/x-repro-node"

type model struct {
	*qt.QAbstractItemModel
	n int
}

func newModel(n int) *model {
	m := &model{QAbstractItemModel: qt.NewQAbstractItemModel(), n: n}
	m.OnRowCount(m.rowCount)
	m.OnColumnCount(m.columnCount)
	m.OnIndex(m.index)
	m.OnParent(m.parent)
	m.OnData(m.data)
	m.OnFlags(m.flags)
	m.OnMimeTypes(m.mimeTypes)
	m.OnMimeData(m.mimeData)
	m.OnDropMimeData(m.dropMimeData)
	m.OnSupportedDropActions(m.supportedDropActions)
	return m
}

func (m *model) rowCount(parent *qt.QModelIndex) int {
	if parent.IsValid() {
		return 0
	}
	return m.n
}
func (m *model) columnCount(parent *qt.QModelIndex) int { return 1 }
func (m *model) index(row, col int, parent *qt.QModelIndex) *qt.QModelIndex {
	idx := m.CreateIndex2(row, col, uintptr(row+1))
	return &idx
}
func (m *model) parent(child *qt.QModelIndex) *qt.QModelIndex { return qt.NewQModelIndex() }
func (m *model) data(index *qt.QModelIndex, role int) *qt.QVariant {
	if role == int(qt.DisplayRole) && index.IsValid() {
		return qt.NewQVariant11("row " + strconv.FormatInt(int64(index.InternalId()), 10))
	}
	return qt.NewQVariant()
}
func (m *model) flags(super func(*qt.QModelIndex) qt.ItemFlag, index *qt.QModelIndex) qt.ItemFlag {
	if !index.IsValid() {
		return qt.ItemIsDropEnabled
	}
	return qt.ItemIsSelectable | qt.ItemIsEnabled | qt.ItemIsDragEnabled | qt.ItemIsDropEnabled
}
func (m *model) mimeTypes(super func() []string) []string { return []string{reproMime} }
func (m *model) supportedDropActions(super func() qt.DropAction) qt.DropAction {
	return qt.MoveAction
}
func (m *model) mimeData(super func([]qt.QModelIndex) *qt.QMimeData, indexes []qt.QModelIndex) *qt.QMimeData {
	md := qt.NewQMimeData()
	if len(indexes) > 0 {
		md.SetData(reproMime, []byte(strconv.FormatInt(int64(indexes[0].InternalId()), 10)))
	}
	return md
}
func (m *model) dropMimeData(super func(*qt.QMimeData, qt.DropAction, int, int, *qt.QModelIndex) bool, data *qt.QMimeData, action qt.DropAction, row, col int, parent *qt.QModelIndex) bool {
	return false
}

func main() {
	qt.NewQApplication(os.Args)
	view := qt.NewQTreeView2()
	view.SetModel(newModel(10).QAbstractItemModel)
	view.SetDragDropMode(qt.QAbstractItemView__InternalMove)
	view.Show()
	qt.QApplication_Exec()
}

Drag any row → crash.

C++ backtrace (gdb catch throw)

#1 qBadAlloc()
#3 QString::fromUtf8(QByteArrayView)
#5 MiqtVirtualQAbstractItemModel::mimeTypes()
#7 QAbstractItemView::dragEnterEvent(QDragEnterEvent*)
...
#25 QDrag::exec → #26 QAbstractItemView::startDrag → #28 mouseMoveEvent

On drag-enter the view calls the model's mimeTypes(). The generated C++
trampoline reads the miqt_array of miqt_string returned by the Go callback
and does QString::fromUtf8(arr[i].data, arr[i].len) — but arr[i].len is
garbage (freed heap), so Qt throws qBadAlloc.

Root cause

In gen_qabstractitemmodel.go, miqt_exec_callback_QAbstractItemModel_mimeTypes:

virtualReturn := gofunc(...)                          // []string
virtualReturn_CArray := (*[...])(C.malloc(...))
defer C.free(unsafe.Pointer(virtualReturn_CArray))    // (a) runs on return
for i := range virtualReturn {
    ...
    virtualReturn_i_ms.data = C.CString(virtualReturn[i])
    virtualReturn_i_ms.len  = C.size_t(len(virtualReturn[i]))
    defer C.free(unsafe.Pointer(virtualReturn_i_ms.data)) // (b) runs on return
    ...
}
return virtualReturn_ma            // (a)/(b) free HERE, before C++ reads it

Generator origin: cmd/genbindings/emitgo.go, the virtual-callback emission
path reuses emitParameterGo2CABIForwarding to marshal the return value —
and that helper emits defer C.free(...). That lifetime is correct for
arguments (freed after the C call returns) but a use-after-free for callback
return values (the C++ caller reads them after the Go callback has returned).

Systemic: every overridden virtual returning a heap CABI type — string,
[]string, QList<T>, maps — is affected. mimeTypes is just the one
exercised first by drag-enter.

The fix

• emitgo.go — stripDeferCFree() removes the defer C.free(...) lines
  from the return-value marshaling, so the CABI memory outlives the Go
  callback's return.
• emitcabi.go — emitCABI2CppFreeReturn() frees that CABI memory in the
  C++ trampoline after copying it into the real Qt return type (each element's
  data + the top-level buffer), mirroring the heap cases of
  emitCABI2CppForwarding.

Verification

• go build ./cmd/genbindings — passes.
• go test ./cmd/genbindings -run TestStripDeferCFree — passes (strips exactly
  the defer C.free lines, leaves the allocations intact).
• Regenerated output (ran the patched generator over qt6) — both sides are
  as intended:
  • gen_qabstractitemmodel.go: the mimeTypes callback now has zero
    defer C.free (was 2).
  • gen_qabstractitemmodel.cpp: the mimeTypes trampoline now frees after the
    copy —

    // ... copy loop builds callback_return_value_QList ...
    struct miqt_string* callback_return_value_free_arr = static_cast<struct miqt_string*>(callback_return_value.data);
    for(size_t i = 0; i < callback_return_value.len; ++i) {
        free(callback_return_value_free_arr[i].data);
    }
    free(callback_return_value.data);
    return callback_return_value_QList;

• Runtime: with the equivalent change applied to the generated files, both
  the minimal repro and a real custom-model app drag/drop cleanly, with
  drag-reparent + save persisting correctly, and no leak.

Note on regenerated files

This PR contains the generator change + a unit test; I haven't committed the
regenerated gen_* for the whole tree. My environment (Qt 6.11 + clang 22 vs
miqt's pinned Qt 6.9 + clang 18) needs allowlist tweaks for a clean full
regenerate, so I've left the committed-file regeneration to your CI /
miqt-docker. I did run a targeted regenerate of the affected file to confirm
the generator emits the correct output on both sides (shown above).

[mappu]

Q: Instead of generating this code and then stripping it out, how about adding a parameter to emitParameterGo2CABIForwarding that controls whether the C.free call is added?

[mappu]

Fixes: #86

[mappu]

Thank you for the PR and for taking a look at the problem! It's a real bug in miqt.

A passing CI will require the branch to include a committed, regenerated genbindings output - it should be possible to use the makefile's make genbindings target to rebuild the bindings in the sealed docker environment, and add the regeneration as a second commit. If you're not able to, i can do it later.

It looks like AI tools were used to help make this PR. That's not a problem necessarily. But can I ask what AI tools are you using?

[5k3105]

opus 4.8

[5k3105]

Done — pushed the regenerated bindings plus a small follow-up. The branch now has three commits:

1. 8c8efab — the original UAF fix.
2. f1bfef7 — genbindings: skip empty free-loop for borrowed-pointer list returns. A small follow-up to the fix above. emitCABI2CppFreeReturn() was emitting the per-element cast + for loop unconditionally; for QList<T> returns whose element has no per-element free (borrowed-pointer types like QModelIndex*), that produced a do-nothing loop plus an unused T* ..._arr local (-Wunused-variable). Now the per-element loop is only emitted when there is actually an element free to do, and the top-level free(callback_return_value.data) is always emitted. No behavioural change — purely drops dead code from the generated output.
3. 9ee62a2 — qt6: regenerate bindings (virtual-callback UAF fix) — the regenerated output you asked for, produced in the sealed docker environment. 411 files, +972/-1323; the delta is purely the free-handoff change (removed defer C.free on the Go side of virtual-override returns, added the matching post-copy frees in the C++ trampolines), with no API drift.

That should get CI green now. Thanks for the quick review!