runtime: windows syscall with callback that grow stack returns wrong value

[alexbrainman]

Apply this diff:

diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go
index 720f70b..1c10977 100644
--- a/src/runtime/syscall_windows_test.go
+++ b/src/runtime/syscall_windows_test.go
@@ -554,3 +554,77 @@ func TestWERDialogue(t *testing.T) {
    // Child process should not open WER dialogue, but return immediately instead.
    cmd.CombinedOutput()
 }
+
+var used byte
+
+func use(buf []byte) {
+   for _, c := range buf {
+       used += c
+   }
+}
+
+func forceStackCopy() (r int) {
+   var f func(int) int
+   f = func(i int) int {
+       var buf [256]byte
+       use(buf[:])
+       if i == 0 {
+           return 0
+       }
+       return i + f(i-1)
+   }
+   r = f(128)
+   return
+}
+
+func TestReturnAfterStackGrowInCallback(t *testing.T) {
+
+   const src = `
+#include <stdint.h>
+
+typedef uintptr_t __stdcall (*callback)(uintptr_t);
+
+uintptr_t cfunc(callback f, uintptr_t n) {
+   return f(n);
+}
+`
+   tmpdir, err := ioutil.TempDir("", "TestReturnAfterStackGrowInCallback")
+   if err != nil {
+       t.Fatal("TempDir failed: ", err)
+   }
+   defer os.RemoveAll(tmpdir)
+
+   srcname := "mydll.c"
+   err = ioutil.WriteFile(filepath.Join(tmpdir, srcname), []byte(src), 0)
+   if err != nil {
+       t.Fatal(err)
+   }
+   outname := "mydll.dll"
+   cmd := exec.Command("gcc", "-shared", "-s", "-Werror", "-o", outname, srcname)
+   cmd.Dir = tmpdir
+   out, err := cmd.CombinedOutput()
+   if err != nil {
+       t.Fatalf("failed to build dll: %v - %v", err, string(out))
+   }
+   dllpath := filepath.Join(tmpdir, outname)
+
+   dll := syscall.MustLoadDLL(dllpath)
+   defer dll.Release()
+
+   proc := dll.MustFindProc("cfunc")
+
+   cb := syscall.NewCallback(func(n uintptr) uintptr {
+       forceStackCopy()
+       return n
+   })
+
+   // Use a new goroutine so that we get a small stack.
+   c := make(chan uintptr)
+   go func() {
+       r, _, _ := proc.Call(cb, 100)
+       c <- r
+   }()
+   if got, want := <-c, uintptr(100); got != want {
+       t.Errorf("got %d want %d", got, want)
+   }
+}

to cb10ff1 and run new test:

C:\>u:\test -test.v -test.run=ReturnAfter
=== RUN TestReturnAfterStackGrowInCallback
--- FAIL: TestReturnAfterStackGrowInCallback (0.12s)
        syscall_windows_test.go:628: got 0 want 100
FAIL

If you comment out call to forceStackCopy, then the test succeeds.

Alex

[rsc]

Is this another case where the pointer is turned into a uintptr and handed to Windows, so that the escape analysis can't see that the pointer has escaped? The solution if so is not to do that.

[alexbrainman]

I suspect so, but it is updated by our assembler asmstdcall function, not Windows. What should I do here? Ho can I pass this information if not on stack?

Alex

[dvyukov]

@alexbrainman You can allocate LibCall in heap (or better make it a part of M struct)

[alexbrainman]

Maybe I will put it into M. We already do something like that for stdcall. What should I do for syscalls inside windows callback? Save / restore them at the callback start / end? Sounds complicated just for return value and error.

Alex

[dvyukov]

Save / restore them at the callback start / end?

Save/restore what?

I would try the following scheme.
Use the existing m.libcall for syscall. But if it a nested syscall (syscall inside of a syscall callback), then allocate libcall on heap.

[alexbrainman]

Save/restore what?

We could save m.libcall value into local stack variable at the start of callback, and restore it on the way out of callback.

I will try think of something. Thank you @dvyukov .

Alex

[gopherbot]

CL https://golang.org/cl/10370 mentions this issue.

[akavel]

Isn't this broken in Go 1.4.* too? I'm having some weird behaviors on Windows (and there's some initial suspicion they might be return-related), when trying to work with WinAPI (e.g. apparently in WinAPI -> callback -> WinAPI -> callback chains). Not sure if they're the same case, but I've just tried tip, and they seem to disappear magically (i.e. works ok).

[alexbrainman]

Sure this is broken in go1.4.*. Similar I had unexplained returned values from Windows GetMessage. There might be others. This also will cause memory corruption, since those correct return values are written somewhere else.

Alex