purego: fix callback crash for arm64 (#117)

crosscall2 clobbers FP in the frame record. Use NOFRAME flag for callbackasm1
to disable the assembler generated epilogue that restores the corrupted FP.
Save and restore FP manually instead.

Co-authored-by: Erdi Chen <erdi@erdi.us>

commit: ebd8567e49db0b4514b146f222a59dd1acd618b7
author: erdichen · 3y ago · 2023-03-27 17:33 UTC
committer: GitHub · 3y ago · 2023-03-27 17:33 UTC
parent: 09d3b2f
signature: GPG Unverified4AEE18F83AFDEB23 The key is unknown

3 files · +100 -8

A callback_test.go +74 -0

@@ -0,0 +1,74 @@
	1	+	// SPDX-License-Identifier: Apache-2.0
	2	+	// SPDX-FileCopyrightText: 2023 The Ebitengine Authors
	3	+
	4	+	//go:build darwin
	5	+
	6	+	package purego_test
	7	+
	8	+	import (
	9	+	"errors"
	10	+	"fmt"
	11	+	"os"
	12	+	"os/exec"
	13	+	"path/filepath"
	14	+	"strings"
	15	+	"testing"
	16	+	"unsafe"
	17	+
	18	+	"github.com/ebitengine/purego"
	19	+	)
	20	+
	21	+	// TestCallGoFromSharedLib is a test that checks for stack corruption on darwin/arm64
	22	+	// when C calls Go code from a non-Go thread in a dynamically loaded share library.
	23	+	func TestCallGoFromSharedLib(t *testing.T) {
	24	+	libFileName := filepath.Join(t.TempDir(), "libcbtest.so")
	25	+	t.Logf("Build %v", libFileName)
	26	+
	27	+	if err := buildSharedLib(libFileName, filepath.Join("libcbtest", "callback.c")); err != nil {
	28	+	t.Fatal(err)
	29	+	}
	30	+	defer os.Remove(libFileName)
	31	+
	32	+	lib, err := purego.Dlopen(libFileName, purego.RTLD_NOW\|purego.RTLD_GLOBAL)
	33	+	if err != nil {
	34	+	t.Fatalf("Dlopen(%q) faled: %v", libFileName, err)
	35	+	}
	36	+
	37	+	var callCallback func(p uintptr, s string) int
	38	+	purego.RegisterLibFunc(&callCallback, lib, "callCallback")
	39	+
	40	+	goFunc := func(cstr *byte, n int) int {
	41	+	s := string(unsafe.Slice(cstr, n))
	42	+	t.Logf("FROM Go: %s\n", s)
	43	+	return 1
	44	+	}
	45	+
	46	+	const want = 10101
	47	+	cb := purego.NewCallback(goFunc)
	48	+	for i := 0; i < 10; i++ {
	49	+	got := callCallback(cb, "a test string")
	50	+	if got != want {
	51	+	t.Fatalf("%d: callCallback() got %v want %v", i, got, want)
	52	+	}
	53	+	}
	54	+	}
	55	+
	56	+	func buildSharedLib(libFile string, sources ...string) error {
	57	+	out, err := exec.Command("go", "env", "CC").Output()
	58	+	if err != nil {
	59	+	return fmt.Errorf("go env error: %w", err)
	60	+	}
	61	+
	62	+	cc := strings.TrimSpace(string(out))
	63	+	if cc == "" {
	64	+	return errors.New("CC not found")
	65	+	}
	66	+
	67	+	args := append([]string{"-shared", "-Wall", "-Werror", "-o", libFile}, sources...)
	68	+	cmd := exec.Command(cc, args...)
	69	+	if out, err := cmd.CombinedOutput(); err != nil {
	70	+	return fmt.Errorf("compile lib: %w\n%q\n%s", err, cmd, string(out))
	71	+	}
	72	+
	73	+	return nil
	74	+	}

@@ -0,0 +1,74 @@
	1	+	// SPDX-License-Identifier: Apache-2.0
	2	+	// SPDX-FileCopyrightText: 2023 The Ebitengine Authors
	3	+
	4	+	//go:build darwin
	5	+
	6	+	package purego_test
	7	+
	8	+	import (
	9	+	"errors"
	10	+	"fmt"
	11	+	"os"
	12	+	"os/exec"
	13	+	"path/filepath"
	14	+	"strings"
	15	+	"testing"
	16	+	"unsafe"
	17	+
	18	+	"github.com/ebitengine/purego"
	19	+	)
	20	+
	21	+	// TestCallGoFromSharedLib is a test that checks for stack corruption on darwin/arm64
	22	+	// when C calls Go code from a non-Go thread in a dynamically loaded share library.
	23	+	func TestCallGoFromSharedLib(t *testing.T) {
	24	+	libFileName := filepath.Join(t.TempDir(), "libcbtest.so")
	25	+	t.Logf("Build %v", libFileName)
	26	+
	27	+	if err := buildSharedLib(libFileName, filepath.Join("libcbtest", "callback.c")); err != nil {
	28	+	t.Fatal(err)
	29	+	}
	30	+	defer os.Remove(libFileName)
	31	+
	32	+	lib, err := purego.Dlopen(libFileName, purego.RTLD_NOW\|purego.RTLD_GLOBAL)
	33	+	if err != nil {
	34	+	t.Fatalf("Dlopen(%q) faled: %v", libFileName, err)
	35	+	}
	36	+
	37	+	var callCallback func(p uintptr, s string) int
	38	+	purego.RegisterLibFunc(&callCallback, lib, "callCallback")
	39	+
	40	+	goFunc := func(cstr *byte, n int) int {
	41	+	s := string(unsafe.Slice(cstr, n))
	42	+	t.Logf("FROM Go: %s\n", s)
	43	+	return 1
	44	+	}
	45	+
	46	+	const want = 10101
	47	+	cb := purego.NewCallback(goFunc)
	48	+	for i := 0; i < 10; i++ {
	49	+	got := callCallback(cb, "a test string")
	50	+	if got != want {
	51	+	t.Fatalf("%d: callCallback() got %v want %v", i, got, want)
	52	+	}
	53	+	}
	54	+	}
	55	+
	56	+	func buildSharedLib(libFile string, sources ...string) error {
	57	+	out, err := exec.Command("go", "env", "CC").Output()
	58	+	if err != nil {
	59	+	return fmt.Errorf("go env error: %w", err)
	60	+	}
	61	+
	62	+	cc := strings.TrimSpace(string(out))
	63	+	if cc == "" {
	64	+	return errors.New("CC not found")
	65	+	}
	66	+
	67	+	args := append([]string{"-shared", "-Wall", "-Werror", "-o", libFile}, sources...)
	68	+	cmd := exec.Command(cc, args...)
	69	+	if out, err := cmd.CombinedOutput(); err != nil {
	70	+	return fmt.Errorf("compile lib: %w\n%q\n%s", err, cmd, string(out))
	71	+	}
	72	+
	73	+	return nil
	74	+	}

A libcbtest/callback.c +13 -0

@@ -0,0 +1,13 @@
	1	+	// SPDX-License-Identifier: Apache-2.0
	2	+	// SPDX-FileCopyrightText: 2023 The Ebitengine Authors
	3	+
	4	+	#include <string.h>
	5	+
	6	+	typedef int (callback)(const char , int);
	7	+
	8	+	int callCallback(const void fp, const char s) {
	9	+	// If the callback corrupts FP, this local variable on the stack will have incorrect value.
	10	+	int sentinel = 10101;
	11	+	((callback)(fp))(s, strlen(s));
	12	+	return sentinel;
	13	+	}

@@ -0,0 +1,13 @@
	1	+	// SPDX-License-Identifier: Apache-2.0
	2	+	// SPDX-FileCopyrightText: 2023 The Ebitengine Authors
	3	+
	4	+	#include <string.h>
	5	+
	6	+	typedef int (callback)(const char , int);
	7	+
	8	+	int callCallback(const void fp, const char s) {
	9	+	// If the callback corrupts FP, this local variable on the stack will have incorrect value.
	10	+	int sentinel = 10101;
	11	+	((callback)(fp))(s, strlen(s));
	12	+	return sentinel;
	13	+	}

M sys_arm64.s +13 -8


@@ -63,7 +63,7 @@ TEXT syscall9X(SB), NOSPLIT, $0
63	63		FMOVD F0, syscall9Args_r2(R2) // save r2
64	64		RET
65	65
66		-	TEXT callbackasm1(SB), NOSPLIT, $208-0
	66	+	TEXT callbackasm1(SB), NOSPLIT\|NOFRAME, $0
67	67		NO_LOCAL_POINTERS
68	68
69	69		// On entry, the trampoline in zcallback_darwin_arm64.s left

@@ -71,18 +71,19 @@ TEXT callbackasm1(SB), NOSPLIT, $208-0
71	71
72	72		// Save callback register arguments R0-R7.
73	73		// We do this at the top of the frame so they're contiguous with stack arguments.
74		-	// The 7*8 setting up R14 looks like a bug but is not: the eighth word
75		-	// is the space the assembler reserved for our caller's frame pointer,
76		-	// but we are not called from Go so that space is ours to use,
77		-	// and we must to be contiguous with the stack arguments.
78		-	MOVD $arg0-(7*8)(SP), R14
	74	+	SUB $(8*8), RSP, R14
79	75		STP (R0, R1), (0*8)(R14)
80	76		STP (R2, R3), (2*8)(R14)
81	77		STP (R4, R5), (4*8)(R14)
82	78		STP (R6, R7), (6*8)(R14)
83	79
	80	+	// Adjust SP by frame size.
	81	+	// crosscall2 clobbers FP in the frame record so only save/restore SP.
	82	+	SUB $(28*8), RSP
	83	+	MOVD R30, (RSP)
	84	+
84	85		// Create a struct callbackArgs on our stack.
85		-	MOVD $cbargs-(18*8+callbackArgs__size)(SP), R13
	86	+	ADD $(callbackArgs__size + 3*8), RSP, R13
86	87		MOVD R12, callbackArgs_index(R13) // callback index
87	88		MOVD R14, R0
88	89		MOVD R0, callbackArgs_args(R13) // address of args vector

@@ -100,7 +101,11 @@ TEXT callbackasm1(SB), NOSPLIT, $208-0
100	101		BL crosscall2(SB)
101	102
102	103		// Get callback result.
103		-	MOVD $cbargs-(18*8+callbackArgs__size)(SP), R13
	104	+	ADD $(callbackArgs__size + 3*8), RSP, R13
104	105		MOVD callbackArgs_result(R13), R0
105	106
	107	+	// Restore SP
	108	+	MOVD (RSP), R30
	109	+	ADD $(28*8), RSP
	110	+
106	111		RET

 	FMOVD F0, syscall9Args_r2(R2) // save r2
 	RET
-TEXT callbackasm1(SB), NOSPLIT, $208-0
+TEXT callbackasm1(SB), NOSPLIT|NOFRAME, $0
 	NO_LOCAL_POINTERS
 	// On entry, the trampoline in zcallback_darwin_arm64.s left
 	// Save callback register arguments R0-R7.
 	// We do this at the top of the frame so they're contiguous with stack arguments.
-	// The 7*8 setting up R14 looks like a bug but is not: the eighth word
+	SUB $(8*8), RSP, R14
-	// is the space the assembler reserved for our caller's frame pointer,
-	// but we are not called from Go so that space is ours to use,
-	// and we must to be contiguous with the stack arguments.
-	MOVD $arg0-(7*8)(SP), R14
 	STP  (R0, R1), (0*8)(R14)
 	STP  (R2, R3), (2*8)(R14)
 	STP  (R4, R5), (4*8)(R14)
 	STP  (R6, R7), (6*8)(R14)
+	// Adjust SP by frame size.
+	// crosscall2 clobbers FP in the frame record so only save/restore SP.
+	SUB $(28*8), RSP
+	MOVD R30, (RSP)
 	// Create a struct callbackArgs on our stack.
-	MOVD $cbargs-(18*8+callbackArgs__size)(SP), R13
+	ADD $(callbackArgs__size + 3*8), RSP, R13
 	MOVD R12, callbackArgs_index(R13)               // callback index
 	MOVD R14, R0
 	MOVD R0, callbackArgs_args(R13)                 // address of args vector
 	BL crosscall2(SB)
 	// Get callback result.
-	MOVD $cbargs-(18*8+callbackArgs__size)(SP), R13
+	ADD $(callbackArgs__size + 3*8), RSP, R13
 	MOVD callbackArgs_result(R13), R0
+	// Restore SP
+	MOVD (RSP), R30
+	ADD $(28*8), RSP
 	RET