builtin: fix Boehm interior pointers and modal GC aborts on Windows (…

vlib/builtin/builtin_d_gcboehm.c.v

@@ -40,6 +40,8 @@ $if dynamic_boehm ? {
 			#flag -I @VEXEROOT/thirdparty/libgc/include
 		} $else {
 			#flag -DGC_WIN32_THREADS=1
+			#flag -DNO_MSGBOX_ON_ERROR=1
+			#flag -DCONSOLE_LOG=1
 			#flag -DGC_BUILTIN_ATOMIC=1
 			#flag -I @VEXEROOT/thirdparty/libgc/include
 			#flag -DALL_INTERIOR_POINTERS=1
@@ -148,6 +150,10 @@ $if dynamic_boehm ? {
 	} $else $if windows {
 		#flag -DGC_NOT_DLL=1
 		#flag -DGC_WIN32_THREADS=1
+		// For the libgc built from source: report like on other platforms, on
+		// stderr, instead of in a modal message box and an `<exe>.gc.log` file.
+		#flag -DNO_MSGBOX_ON_ERROR=1
+		#flag -DCONSOLE_LOG=1
 		#flag -luser32
 		$if tinyc {
 			#flag -DGC_BUILTIN_ATOMIC=1
@@ -185,6 +191,9 @@ $if gcboehm_leak ? {
 #include <gc.h>
 #include "@VEXEROOT/vlib/builtin/gc_debugger_linux.h"
 #define v_gc_set_warn_proc(cb) GC_set_warn_proc((GC_warn_proc)(cb))
+#define v_gc_set_abort_func(cb) GC_set_abort_func((GC_abort_func)(cb))
+#define v_gc_get_abort_func() ((void *)GC_get_abort_func())
+#define v_gc_call_abort_func(fn, msg) ((GC_abort_func)(fn))(msg)
 
 // #include <gc/gc_mark.h>
 
@@ -296,7 +305,18 @@ pub type FnGC_WarnCB = fn (const_msg &char, arg usize)
 fn C.GC_get_warn_proc() FnGC_WarnCB
 fn C.v_gc_set_warn_proc(cb FnGC_WarnCB)
 
-fn C.GC_register_displacement(offset usize)
+fn C.GC_REGISTER_DISPLACEMENT(offset usize)
+
+type FnGC_AbortCB = fn (const_msg &char)
+
+// The abort handler functions go through casting macros: V function types drop
+// the `const` of Boehm's `const char *` parameter, which gcc rejects.
+fn C.v_gc_get_abort_func() voidptr
+fn C.v_gc_set_abort_func(cb FnGC_AbortCB)
+fn C.v_gc_call_abort_func(cb voidptr, msg &char)
+
+// Boehm's own abort handler, kept for the non-display part of its work.
+__global gc_boehm_default_abort_func voidptr
 
 // gc_get_warn_proc returns the current callback fn, that will be used for printing GC warnings.
 pub fn gc_get_warn_proc() FnGC_WarnCB {
@@ -311,6 +331,26 @@ pub fn gc_set_warn_proc(cb FnGC_WarnCB) {
 // used by builtin_init:
 fn internal_gc_warn_proc_none(const_msg &char, arg usize) {}
 
+// gc_report_fatal_errors_on_stderr replaces Boehm's abort handler. On Windows the
+// default one shows a modal message box and waits for someone to dismiss it, so a
+// console program or a test run hangs instead of failing.
+fn gc_report_fatal_errors_on_stderr() {
+	gc_boehm_default_abort_func = C.v_gc_get_abort_func()
+	C.v_gc_set_abort_func(internal_gc_abort_to_stderr)
+}
+
+// internal_gc_abort_to_stderr prints a fatal Boehm error on stderr, as Boehm does
+// on Linux and macOS. Boehm then ends the process when this returns.
+fn internal_gc_abort_to_stderr(const_msg &char) {
+	// With a nil message the default handler only disables the at-exit leak
+	// collection (and honours GC_LOOP_ON_ABORT); it shows no message box.
+	C.v_gc_call_abort_func(gc_boehm_default_abort_func, unsafe { nil })
+	if const_msg != unsafe { nil } {
+		C.fprintf(C.stderr, c'%s\n', const_msg)
+		C.fflush(C.stderr)
+	}
+}
+
 @[markused]
 fn gc_prepare_for_debugger_init() bool {
 	$if linux {

vlib/builtin/gc_startup_d_gcboehm.c.v

@@ -3,6 +3,7 @@ module builtin
 fn C.GC_INIT()
 fn C.GC_is_init_called() int
 fn C.GC_set_find_leak(int)
+fn C.GC_set_all_interior_pointers(int)
 fn C.GC_set_pages_executable(int)
 fn C.GC_set_free_space_divisor(usize)
 fn C.GC_enable_incremental()
@@ -16,6 +17,15 @@ fn v3_gcboehm_runtime_init() {
 	}
 	debugger_workaround := gc_prepare_for_debugger_init()
 	C.GC_set_pages_executable(0)
+	host_initialized_gc := C.GC_is_init_called() != 0
+	if !host_initialized_gc {
+		// V reaches array data through pointers into the middle of a block (past
+		// the array header, and anywhere for slices), including from heap
+		// objects, so Boehm must recognise interior pointers. Every libgc V builds
+		// from source enables this; the prebuilt one linked on Windows with tcc
+		// does not, and it then frees blocks that live arrays still use.
+		C.GC_set_all_interior_pointers(1)
+	}
 	$if gcboehm_opt ? {
 		// Preserve an already-initialized host collector's process-wide tuning.
 		// GC_INIT() below is a no-op in that case and cannot re-read the env var.
@@ -25,8 +35,19 @@ fn v3_gcboehm_runtime_init() {
 	}
 	C.GC_INIT()
 	// V arrays keep an interior pointer one pointer-width past the allocation
-	// header. Register that displacement so Boehm retains the allocation.
-	C.GC_register_displacement(sizeof(voidptr))
+	// header. Register that displacement so Boehm retains the allocation even
+	// when interior pointers are off (a host that initialized the collector
+	// first). Use the macro: under `-gc boehm_leak` (GC_DEBUG) every object also
+	// starts after Boehm's debug header, and only GC_REGISTER_DISPLACEMENT
+	// registers the offset that header adds.
+	C.GC_REGISTER_DISPLACEMENT(sizeof(voidptr))
+	$if windows {
+		// Leave a host collector's abort handler alone. Installed after GC_INIT,
+		// because the setter takes Boehm's allocator lock.
+		if !host_initialized_gc {
+			gc_report_fatal_errors_on_stderr()
+		}
+	}
 	gc_restore_roots_after_debugger_init(debugger_workaround)
 	$if gcboehm_incr ? {
 		C.GC_enable_incremental()

vlib/v/tests/gc_boehm_abort_reports_to_stderr_test.v

@@ -0,0 +1,63 @@
+// Regression test for the hang in issue #28896. On Windows, Boehm's default
+// fatal-error handler shows a modal message box and waits for someone to click
+// it, so a console program or a test run stops instead of failing. V installs
+// its own handler, which reports the error on stderr like other platforms do.
+//
+// The child program triggers a deterministic Boehm abort: under
+// `-gc boehm_leak`, GC_FREE of a pointer Boehm does not own aborts with
+// "Invalid pointer passed to free()". The child is run with a deadline, so a
+// modal dialog shows up as a timeout failure rather than a hung test.
+import os
+import time
+
+const vexe = @VEXE
+
+const child_deadline = 60 * time.second
+
+fn test_boehm_abort_is_reported_on_stderr_without_blocking() {
+	dir := os.join_path(os.vtmp_dir(), 'v_gc_abort_stderr_${os.getpid()}')
+	os.mkdir_all(dir) or { panic(err) }
+	defer {
+		os.rmdir_all(dir) or {}
+	}
+	source := os.join_path(dir, 'gc_abort_child.v')
+	os.write_file(source, [
+		'fn C.GC_FREE(voidptr)',
+		'',
+		'fn main() {',
+		'\tmut not_heap := 0',
+		'\tunsafe { C.GC_FREE(voidptr(&not_heap)) }',
+		"\tprintln('not reached')",
+		'}',
+	].join('\n')) or { panic(err) }
+	mut exe := os.join_path(dir, 'gc_abort_child')
+	$if windows {
+		exe += '.exe'
+	}
+	build := os.execute('${os.quoted_path(vexe)} -gc boehm_leak -o ${os.quoted_path(exe)} ${os.quoted_path(source)}')
+	if build.exit_code != 0 && build.output.contains('libgc') {
+		eprintln('skipping: no Boehm GC library available\n${build.output}')
+		return
+	}
+	assert build.exit_code == 0, build.output
+
+	mut child := os.new_process(exe)
+	child.set_redirect_stdio()
+	child.run()
+	started := time.now()
+	for child.is_alive() {
+		if time.since(started) > child_deadline {
+			child.signal_kill()
+			child.wait()
+			assert false, 'the Boehm abort blocked for more than ${child_deadline} (a modal dialog?)'
+		}
+		time.sleep(50 * time.millisecond)
+	}
+	child.wait()
+	stdout := child.stdout_slurp()
+	stderr := child.stderr_slurp()
+	child.close()
+	assert child.code != 0, 'the child exited cleanly; stdout: ${stdout}'
+	assert !stdout.contains('not reached')
+	assert stderr.contains('Invalid pointer passed to free()'), stderr
+}

vlib/v/tests/gc_boehm_interior_pointers_test.v

@@ -0,0 +1,19 @@
+// Regression test for issue #28896. V reaches array data through pointers into
+// the middle of a Boehm block (past the array header, and anywhere for slices),
+// including from heap objects, so the runtime must have Boehm recognise interior
+// pointers. The prebuilt libgc linked on Windows with tcc leaves that off, and a
+// collection then freed blocks that live arrays and slices still used.
+//
+// This checks the setting itself: whether a particular block survives a
+// collection depends on stale values in the conservatively scanned stack, which
+// can keep a block alive by accident and make such a test pass without the fix.
+fn C.GC_get_all_interior_pointers() int
+
+fn test_boehm_recognises_interior_pointers() {
+	$if gcboehm ? {
+		assert C.GC_get_all_interior_pointers() == 1
+	} $else {
+		eprintln('skipping: not a Boehm GC build')
+		assert true
+	}
+}

vlib/v/tests/gc_boehm_leak_array_displacement_test.v

@@ -0,0 +1,98 @@
+// vtest vflags: -gc boehm_leak
+// Regression test for issue #28896. A managed V array points one header past the
+// start of its Boehm block, and `-gc boehm_leak` builds Boehm with GC_DEBUG,
+// which puts its own debug header in front of every object as well. The
+// prebuilt libgc linked on Windows with tcc does not recognise interior pointers
+// by default, so a collection freed array blocks that a heap object still used,
+// and the next free() of one aborted with "Invalid pointer passed to free()".
+fn C.GC_base(voidptr) voidptr
+fn C.GC_size(voidptr) usize
+fn C.GC_gcollect()
+fn C.GC_noop1(u64)
+fn C.GC_get_all_interior_pointers() int
+
+const block_ints = 2048
+
+@[heap]
+struct ArrayHolder {
+mut:
+	items []int
+}
+
+// The array gets a Boehm large block of its own, so a dropped block cannot hide
+// behind live neighbours.
+@[noinline]
+fn new_array_holder() &ArrayHolder {
+	mut holder := &ArrayHolder{}
+	for i in 0 .. block_ints {
+		holder.items << i
+	}
+	return holder
+}
+
+// clear_stack_residue overwrites the dead stack below the caller. Boehm scans
+// the stack conservatively, and a stale copy of a pointer into the array block,
+// left behind by building the array, would otherwise keep the block alive by
+// itself and hide the bug.
+@[noinline]
+fn clear_stack_residue() {
+	mut scratch := [1024]u64{}
+	unsafe {
+		C.memset(&scratch[0], 0, sizeof(scratch))
+		// Keeps the compiler from dropping the zeroing as a dead store.
+		C.GC_noop1(u64(&scratch[0]))
+	}
+}
+
+@[noinline]
+fn churn_allocations() {
+	for i in 0 .. 64 {
+		mut junk := []u8{len: 256}
+		junk[0] = u8(i)
+	}
+}
+
+// array_block_intact reports whether the holder's array is still backed by the
+// large Boehm block it was allocated in: Boehm must still know a block there, of
+// at least the array's size, starting just before V's data (V's array header
+// plus Boehm's debug header). A dropped block is either unknown to GC_base or
+// has been reused for smaller objects.
+@[noinline]
+fn array_block_intact(holder &ArrayHolder) bool {
+	block := unsafe { voidptr(u64(holder.items.data) - u64(holder.items.offset)) }
+	base := C.GC_base(block)
+	if base == unsafe { nil } {
+		return false
+	}
+	return C.GC_size(base) >= usize(block_ints * sizeof(int)) && u64(block) - u64(base) < 256
+}
+
+fn test_boehm_leak_recognises_interior_pointers() {
+	$if gcboehm_leak ? {
+		assert C.GC_get_all_interior_pointers() == 1
+	} $else {
+		eprintln('skipping: not a -gc boehm_leak build')
+		assert true
+	}
+}
+
+fn test_boehm_leak_collection_keeps_a_heap_held_array_block() {
+	$if gcboehm_leak ? {
+		holder := new_array_holder()
+		clear_stack_residue()
+		for _ in 0 .. 3 {
+			churn_allocations()
+			C.GC_gcollect()
+		}
+		// Checked through GC_base rather than by freeing, so a dropped block
+		// fails an assertion instead of reaching Boehm's abort.
+		assert array_block_intact(holder)
+		assert holder.items.flags.has(.managed)
+		assert holder.items.len == block_ints
+		assert holder.items[0] == 0
+		assert holder.items[block_ints - 1] == block_ints - 1
+	} $else {
+		eprintln('skipping: not a -gc boehm_leak build')
+		assert true
+	}
+}