syscall: add NewCallbackCDecl definition #6338
Labels
Comments
Adding to the .goc file (which is a C file in disguise) implements a function but does not make it available for use by Go programs. To do that you have to add a prototype (without a function body) in a .go file. I would imagine you need to define NewCallbackCDecl in syscall/syscall_windows.go next to NewCallback. Labels changed: added priority-later, go1.2, removed priority-triage. Status changed to Accepted. |
Thank you, Russ. I know how to make it available for use by Go programs. But before I do, I would like to know how people use it. Also I would like to have a test so we can satisfy ourselves that it actually works, and so we can keep it working in the future. Alex Status changed to WaitingForReply. |
Just realised "C" is a no-go from "testing" tests. Do as you must. I raised the issue and pointed at the solution but cannot seek to mandate its implementation based on a single need in years. I am developing swigless generic marshalling using reflect MakeFunc and for the sake of completeness would like it but can find a way around if it is removed. Hopefully code review has improved since back then. |
Just realised "C" is a no-go from "testing" tests. Do as you must. I raised the issue and pointed at the solution but cannot seek to mandate its implementation based on a single need in years. I am developing swigless generic marshalling using reflect MakeFunc and for the sake of completeness would like it but can find a way around if it is removed. Hopefully code review has improved since back then. |
func compare(i1, i2 int) int {
return i1 - i2
}
...
c := syscall.NewCallbackCDecl(compare)
for i := 1; i < 4; i++ {
r,_,_:=syscall.Syscall(c,2,uintptr(i),2,0)
if int(r) != i-2 {
t.Fatal...
...
Tests the mechanism but does not prove stack use and restoration.
Layout courtesy of no gofmt on this kindle
:)
|
Perhaps we don't understand each other, so lets try again. You have asked to export runtime.NewCallbackCDecl. No one uses that function yet. So, I would like to know, how you are planning to use it. So we could devise a test to make sure we don't break that functionality in the future. Alternatively, you can enable it yourself as per Russ's instructions. And we don't have to do anything here. Alex |
1. By syscall.Syscall transparently catering for cdecl and stdcall external calls,
perhaps a false sense of security exists with NewCallback.
2. Human nature being what it is the "don't-read-the-manual" group is probably much
larger than group who do.
3. The Stdcall vs CDecl issue is arcane to a lot of people.
I give an example of incorrect use of NewCallback for a CDecl callback in [1]. The
program runs successfully and the calls-back do as they should. (The dlls GObject and
Gtk can be found in any windows system set up to use pprof with dot/Graphviz)
There may already be many programs in the wild doing the same thing.
Why the program doesn't fail is quite simple. Go program calls dll which in turn calls
the Go callback. Go callback should cut its arguments from the stack before returning
but doesn't. As most callbacks are a simple give and take operation, the dll and the Go
caller, by virtue of sp restoration don't even notice.
For enumeration type callbacks, where Go calls dll and dll has N>1 conversations with
the Go callback, the intra-dll stack integrity becomes a problem.
I include a demonstration dll [2][3], caller go package [4][5] and output [6.1][6.2][6.3]
[6.1] shows that calls with 2-9 arguments can be called a varying number of times by the
dll without failure. The threshold for each arity is different.
[6.2] shows the threshold of arity-3to9 being broken by 1 in each case.
[6.3] shows the effect of changing the optimization of the dll on the arity-2 callback,
which otherwise didn't fail.
It is my recommendation that you:
- add NewCallbackCDecl
or - strongly redocument NewCallback
or - remove NewCallback
Glad I found out how to call Go from Go! :)
============== [1] example.go ===============
package main
import u "unsafe"
import f "fmt"
import s "syscall"
var (
do = s.MustLoadDLL("libgobject-2.0-0.dll")
dg = s.MustLoadDLL("libgtk-win32-2.0-0.dll")
g_signal_connect_data = do.MustFindProc("g_signal_connect_data")
gtk_button_new_with_label = dg.MustFindProc("gtk_button_new_with_label")
gtk_container_add = dg.MustFindProc("gtk_container_add")
gtk_container_set_border_width = dg.MustFindProc("gtk_container_set_border_width")
gtk_init = dg.MustFindProc("gtk_init")
gtk_main = dg.MustFindProc("gtk_main")
gtk_main_quit = dg.MustFindProc("gtk_main_quit")
gtk_widget_show = dg.MustFindProc("gtk_widget_show")
gtk_window_new = dg.MustFindProc("gtk_window_new")
gtk_widget_destroy = dg.MustFindProc("gtk_widget_destroy").Addr()
)
type void *struct{} // for bogus return values
func deleteEvent(a1, a2, a3 uintptr) uintptr {
f.Println("delete event occurred with args", a1, a2, a3)
return 0
}
func destroy(a1, a2 uintptr) (_ void) {
// actually void return but syscall demands a return !!!
f.Println("destroy event occurred with args", a1, a2)
gtk_main_quit.Call()
return
}
func hello(a1, a2 uintptr) (_ void) {
// actually void return but syscall demands a return !!!
f.Println("clicked event occurred with args", a1, a2)
return
}
func cstring(i string) uintptr {
r, _ := s.BytePtrFromString(i)
return (uintptr)(u.Pointer(r))
}
func main() {
defer do.Release()
defer dg.Release()
gtk_init.Call(0, 0)
ncb := s.NewCallback // Should be s.NewCallbackCDecl
W, _, _ := gtk_window_new.Call(0)
//g_signal_connect(A,B,C,D) :-
// g_signal_connect_data(A,B,C,D,0,0).
g_signal_connect_data.Call(
W, cstring("delete-event"), ncb(deleteEvent), 123, 0, 0)
g_signal_connect_data.Call(
W, cstring("destroy"), ncb(destroy), 456, 0, 0)
gtk_container_set_border_width.Call(W, 30)
B, _, _ := gtk_button_new_with_label.Call(
cstring(" Hello World "))
clicked := cstring("clicked")
g_signal_connect_data.Call(
B, clicked, ncb(hello), 789, 0, 0)
//g_signal_connect_swapped(A,B,C,D) :-
// g_signal_connect_data(A,B,C,D,0,2).
g_signal_connect_data.Call(
B, clicked, gtk_widget_destroy, W, 0, 2)
gtk_container_add.Call(W, B)
gtk_widget_show.Call(B)
gtk_widget_show.Call(W)
gtk_main.Call()
}
============== [2] faux.c ===============
// __cdecl is default
typedef void (*ccb2)(int,int);
typedef void __stdcall (*stdcb2)(int,int);
typedef void (*ccb3)(int,int,int);
typedef void __stdcall (*stdcb3)(int,int,int);
typedef void (*ccb4)(int,int,int,int);
typedef void __stdcall (*stdcb4)(int,int,int,int);
typedef void (*ccb5)(int,int,int,int,int);
typedef void __stdcall (*stdcb5)(int,int,int,int,int);
typedef void (*ccb6)(int,int,int,int,int,int);
typedef void __stdcall (*stdcb6)(int,int,int,int,int,int);
typedef void (*ccb7)(int,int,int,int,int,int,int);
typedef void __stdcall (*stdcb7)(int,int,int,int,int,int,int);
typedef void (*ccb8)(int,int,int,int,int,int,int,int);
typedef void __stdcall (*stdcb8)(int,int,int,int,int,int,int,int);
typedef void (*ccb9)(int,int,int,int,int,int,int,int,int);
typedef void __stdcall (*stdcb9)(int,int,int,int,int,int,int,int,int);
void c2(ccb2 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2);
}
}
void std2(stdcb2 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2);
}
}
void c3(ccb3 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3);
}
}
void std3(stdcb3 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3);
}
}
void c4(ccb4 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4);
}
}
void std4(stdcb4 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4);
}
}
void c5(ccb5 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5);
}
}
void std5(stdcb5 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5);
}
}
void c6(ccb6 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6);
}
}
void std6(stdcb6 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6);
}
}
void c7(ccb7 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7);
}
}
void std7(stdcb7 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7);
}
}
void c8(ccb8 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7,8);
}
}
void std8(stdcb8 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7,8);
}
}
void c9(ccb9 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7,8,9);
}
}
void std9(stdcb9 f, int n) {
int i;
for(i=0;i<n;i++){
f(1,2,3,4,5,6,7,8,9);
}
}
============== [3] makedlls.bat ===============
gcc -shared -s -o faux.dll faux.c
gcc -shared -s -o fauxO2.dll -O2 faux.c
============== [4] test.go ===============
package test
============== [5] test_test.go ===============
package test
import (
. "fmt"
. "syscall"
"testing"
)
/*
//bug?
//you can have:
func a() (_ int) { return }
//or:
var b = func() (_ int) { return }
//but not:
func c() {
var d = func() (_ int) { return }
}
*/
//Callbacks must have a return!!! Unfortunate but true.
type void *struct{}
const dll = "faux.dll"
func test(n uintptr, extc, extstd string, ca, stda uintptr) {
d := MustLoadDLL(dll)
defer d.Release()
d.MustFindProc(extstd).Call(stda, n)
Println("Stdcall done")
d.MustFindProc(extc).Call(ca, n)
Println("CDecl done")
}
func TestCallbackIsUsed(t *testing.T) {
n := uintptr(1)
c := func(i1, i2 int) void {
Println("in callback")
if i1+i2 != 3 || i1 != i2-1 {
t.Error("bad input")
}
return nil
}
ca := NewCallbackCDecl(c)
s := func(i1, i2 int) void {
Println("in callback")
if i1+i2 != 3 || i1 != i2-1 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 2-arg correctly using NewCallbackCDecl")
test(n, "c2", "std2", ca, stda)
}
func TestNx2(t *testing.T) {
n := uintptr(10000)
c := func(i1, i2 int) void {
if i1+i2 != 3 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2 int) void {
if i1+i2 != 3 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 2-arg incorrectly using NewCallback")
test(n, "c2", "std2", ca, stda)
}
func TestNx3(t *testing.T) {
n := uintptr(3)
c := func(i1, i2, i3 int) void {
if i1+i2+i3 != 6 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3 int) void {
if i1+i2+i3 != 6 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 3-arg CDecl incorrectly using NewCallback")
test(n, "c3", "std3", ca, stda)
}
func TestNx4(t *testing.T) {
n := uintptr(5)
c := func(i1, i2, i3, i4 int) void {
if i1+i2+i3+i4 != 10 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4 int) void {
if i1+i2+i3+i4 != 10 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 4-arg CDecl incorrectly using NewCallback")
test(n, "c4", "std4", ca, stda)
}
func TestNx5(t *testing.T) {
n := uintptr(2)
c := func(i1, i2, i3, i4, i5 int) void {
if i1+i2+i3+i4+i5 != 15 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4, i5 int) void {
if i1+i2+i3+i4+i5 != 15 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 5-arg CDecl incorrectly using NewCallback")
test(n, "c5", "std5", ca, stda)
}
func TestNx6(t *testing.T) {
n := uintptr(7)
c := func(i1, i2, i3, i4, i5, i6 int) void {
if i1+i2+i3+i4+i5+i6 != 21 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4, i5, i6 int) void {
if i1+i2+i3+i4+i5+i6 != 21 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 6-arg CDecl incorrectly using NewCallback")
test(n, "c6", "std6", ca, stda)
}
func TestNx7(t *testing.T) {
n := uintptr(6)
c := func(i1, i2, i3, i4, i5, i6, i7 int) void {
if i1+i2+i3+i4+i5+i6+i7 != 28 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4, i5, i6, i7 int) void {
if i1+i2+i3+i4+i5+i6+i7 != 28 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 7-arg CDecl incorrectly using NewCallback")
test(n, "c7", "std7", ca, stda)
}
func TestNx8(t *testing.T) {
n := uintptr(1)
c := func(i1, i2, i3, i4, i5, i6, i7, i8 int) void {
if i1+i2+i3+i4+i5+i6+i7+i8 != 36 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4, i5, i6, i7, i8 int) void {
if i1+i2+i3+i4+i5+i6+i7+i8 != 36 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 8-arg CDecl incorrectly using NewCallback")
test(n, "c8", "std8", ca, stda)
}
func TestNx9(t *testing.T) {
n := uintptr(8)
c := func(i1, i2, i3, i4, i5, i6, i7, i8, i9 int) void {
if i1+i2+i3+i4+i5+i6+i7+i8+i9 != 45 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
s := func(i1, i2, i3, i4, i5, i6, i7, i8, i9 int) void {
if i1+i2+i3+i4+i5+i6+i7+i8+i9 != 45 {
t.Error("bad input")
}
return nil
}
stda := NewCallback(s)
Println(dll, n, "x 9-arg CDecl incorrectly using NewCallback")
test(n, "c9", "std9", ca, stda)
}
func TestLooped1x9(t *testing.T) {
d := MustLoadDLL(dll)
defer d.Release()
dc := d.MustFindProc("c9").Addr()
c := func(i1, i2, i3, i4, i5, i6, i7, i8, i9 int) void {
if i1+i2+i3+i4+i5+i6+i7+i8+i9 != 45 {
t.Error("bad input")
}
return nil
}
ca := NewCallback(c)
n := 10000
Println(dll, n, "iterations of 1 x 9-arg CDecl incorrectly using NewCallback")
for i := 0; i < n; i++ {
Syscall(dc, 2, ca, 1, 0)
}
}
============== [6.1] test output faux.dll ===============
=== RUN TestCallbackIsUsed
../faux.dll 1 x 2-arg correctly using NewCallbackCDecl
in callback
Stdcall done
in callback
CDecl done
--- PASS: TestCallbackIsUsed (0.17 seconds)
=== RUN TestNx2
../faux.dll 10000 x 2-arg incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx2 (0.02 seconds)
=== RUN TestNx3
../faux.dll 3 x 3-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx3 (0.00 seconds)
=== RUN TestNx4
../faux.dll 5 x 4-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx4 (0.00 seconds)
=== RUN TestNx5
../faux.dll 2 x 5-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx5 (0.00 seconds)
=== RUN TestNx6
../faux.dll 7 x 6-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx6 (0.00 seconds)
=== RUN TestNx7
../faux.dll 6 x 7-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx7 (0.00 seconds)
=== RUN TestNx8
../faux.dll 1 x 8-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx8 (0.00 seconds)
=== RUN TestNx9
../faux.dll 8 x 9-arg CDecl incorrectly using NewCallback
Stdcall done
CDecl done
--- PASS: TestNx9 (0.00 seconds)
=== RUN TestLooped1x9
../faux.dll 10000 iterations of 1 x 9-arg CDecl incorrectly using NewCallback
--- PASS: TestLooped1x9 (0.00 seconds)
PASS
ok github.com/tHinqa/goIssue6338/test 0.312s
============== [6.2] test output faux.dll failures ===============
=== RUN TestNx3
../faux.dll 4 x 3-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
=== RUN TestNx4
../faux.dll 6 x 4-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
=== RUN TestNx5
../faux.dll 3 x 5-arg CDecl incorrectly using NewCallback
Stdcall done
!!!!!!!!!!!! No failure occurs !!!!!!!!!!!!! Program simply exited
=== RUN TestNx6
../faux.dll 8 x 6-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
=== RUN TestNx7
../faux.dll 7 x 7-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
=== RUN TestNx8
../faux.dll 2 x 8-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
=== RUN TestNx9
../faux.dll 9 x 9-arg CDecl incorrectly using NewCallback
Stdcall done
fatal error: malloc/free - deadlock
[signal 0xc0000005 code=0x1 addr=0x2f0 pc=0x41ab28]
============== [6.3] test output fauxO2.dll ===============
=== RUN TestNx2
../fauxO2.dll 10000 x 2-arg incorrectly using NewCallback
Stdcall done
exit status -1073741819
FAIL github.com/tHinqa/goIssue6338/test 0.359s
|
> i don't understand what's going on here. ... I am trying to decide if NewCallbackCDecl is that useful. I think the fact that it is used by some products - "... (The dlls GObject and Gtk can be found in any windows system set up to use pprof with dot/Graphviz) ..." (as per https://golang.org/issue/6338?c=11), means we should export it for use. I will try to use some of kin.wilson.za code to build a test for NewCallbackCDecl, so we don't break it in the future. > ... if we're not going to export > NewCallbackCDecl we should delete the new code. if it can't be called, why > do we have it? Looks like hector created the code https://golang.org/cl/4058046. And then webmaster asked me to "export" the function https://golang.org/cl/4969047. Goes to show how much he used it, as it still doesn't work. As to the code itself, I don't think, there is much there to "remove" - callback code is pretty general now. Alex Status changed to Thinking. |
|
This issue was closed by revision 4216203. Status changed to Fixed. |
kin.wilson.za, Here https://golang.org/cl/36180044/ is the change with your test. Please, review. Thank you. Alex Status changed to Started. |
|
This issue was closed by revision 7f8a505. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by kin.wilson.za:
The text was updated successfully, but these errors were encountered: