runtime: bad pointer!

[dvyukov]

Hit by perf dashboard no windows/amd64 build benchmark:
http://build.golang.org/log/e8097da06d42ee056625e20ec6ec0be46fb3ea6a
on revision:
https://code.google.com/p/go/source/detail?r=b0e30135710173e2af2140fc019d408bcd1b45ab

runtime: bad pointer in frame code.google.com/p/go.benchmarks/driver.func·003 at
0xc082a99f28: 0xa90
fatal error: bad pointer!

runtime stack:
runtime.throw(0x8ec8b2)
    c:/go/src/runtime/panic.go:465 +0xad fp=0x270fbe0 sp=0x270fbb0
adjustpointers(0xc082a99f28, 0x270fc98, 0x270fe38, 0x835a88)
    c:/go/src/runtime/stack.c:407 +0x33b fp=0x270fc48 sp=0x270fbe0
adjustframe(0x270fd68, 0x270fe38, 0x101)
    c:/go/src/runtime/stack.c:512 +0x1a5 fp=0x270fcd0 sp=0x270fc48
runtime.gentraceback(0x40c540, 0xc082a99228, 0x0, 0xc082a96480, 0x0, 0x0, 0x7fffffff,
0x270fe28, 0x270fe38, 0x434200, ...)
    c:/go/src/runtime/traceback.go:295 +0x778 fp=0x270fdc0 sp=0x270fcd0
copystack(0xc082a96480, 0x4000)
    c:/go/src/runtime/stack.c:621 +0x17a fp=0x270fe80 sp=0x270fdc0
runtime.newstack()
    c:/go/src/runtime/stack.c:774 +0x5f0 fp=0x270ff30 sp=0x270fe80
runtime.morestack()
    c:/go/src/runtime/asm_amd64.s:324 +0x86 fp=0x270ff38 sp=0x270ff30

goroutine 35 [stack growth]:
runtime.mallocgc(0xf, 0x671bc0, 0x1, 0x0)
    c:/go/src/runtime/malloc.go:46 fp=0xc082a99230 sp=0xc082a99228
runtime.newarray(0x671bc0, 0xf, 0x0)
    c:/go/src/runtime/malloc.go:365 +0xca fp=0xc082a99268 sp=0xc082a99230
runtime.makeslice(0x665b60, 0xf, 0xf, 0x0, 0x0, 0x0)
    c:/go/src/runtime/slice.go:32 +0x163 fp=0xc082a992b0 sp=0xc082a99268
syscall.ByteSliceFromString(0x7696f0, 0xe, 0x0, 0x0, 0x0, 0x0, 0x0)
    c:/go/src/syscall/syscall.go:51 +0x137 fp=0xc082a99330 sp=0xc082a992b0
syscall.BytePtrFromString(0x7696f0, 0xe, 0x0, 0x0, 0x0)
    c:/go/src/syscall/syscall.go:65 +0x4b fp=0xc082a99370 sp=0xc082a99330
syscall.(*DLL).FindProc(0xc082006380, 0x7696f0, 0xe, 0x0, 0x0, 0x0)
    c:/go/src/syscall/dll_windows.go:70 +0x62 fp=0xc082a99480 sp=0xc082a99370
syscall.(*LazyProc).Find(0xc08200c270, 0x0, 0x0)
    c:/go/src/syscall/dll_windows.go:243 +0x11e fp=0xc082a994c8 sp=0xc082a99480
syscall.(*LazyProc).mustFind(0xc08200c270)
    c:/go/src/syscall/dll_windows.go:257 +0x2f fp=0xc082a99500 sp=0xc082a994c8
syscall.(*LazyProc).Addr(0xc08200c270, 0x0)
    c:/go/src/syscall/dll_windows.go:266 +0x2f fp=0xc082a99510 sp=0xc082a99500
syscall.FormatMessage(0x3200, 0x40900000057, 0xc082b4b5e8, 0x12c, 0x12c, 0x0, 0x0, 0x0,
0x0)
    c:/go/src/syscall/zsyscall_windows.go:241 +0x7e fp=0xc082a99598 sp=0xc082a99510
syscall.Errno.Error(0x57, 0x0, 0x0)
    c:/go/src/syscall/syscall_windows.go:87 +0x140 fp=0xc082a998b0 sp=0xc082a99598
syscall.(*Errno).Error(0xc082a89150, 0x0, 0x0)
    <autogenerated>:1 +0xae fp=0xc082a998e8 sp=0xc082a998b0
fmt.(*pp).handleMethods(0xc082ab6340, 0x76, 0x0, 0x100)
    c:/go/src/fmt/print.go:692 +0x40a fp=0xc082a999a8 sp=0xc082a998e8
fmt.(*pp).printArg(0xc082ab6340, 0x708280, 0xc082a89150, 0x76, 0x0, 0x0)
    c:/go/src/fmt/print.go:790 +0x4c3 fp=0xc082a99aa0 sp=0xc082a999a8
fmt.(*pp).doPrintf(0xc082ab6340, 0x786350, 0x16, 0xc082b4bf80, 0x1, 0x1)
    c:/go/src/fmt/print.go:1165 +0x21ba fp=0xc082a99e30 sp=0xc082a99aa0
fmt.Sprintf(0x786350, 0x16, 0xc082b4bf80, 0x1, 0x1, 0x0, 0x0)
    c:/go/src/fmt/print.go:203 +0x7f fp=0xc082a99e80 sp=0xc082a99e30
log.Printf(0x786350, 0x16, 0xc082b4bf80, 0x1, 0x1)
    c:/go/src/log/log.go:276 +0x57 fp=0xc082a99ed0 sp=0xc082a99e80
code.google.com/p/go.benchmarks/driver.func·003()
    c:/src/perfer/work/gopath/src/code.google.com/p/go.benchmarks/driver/driver_windows.go:114 +0x32b fp=0xc082a99fe0 sp=0xc082a99ed0
runtime.goexit()
    c:/go/src/runtime/proc.c:1651 fp=0xc082a99fe8 sp=0xc082a99fe0
created by code.google.com/p/go.benchmarks/driver.initJob
    c:/src/perfer/work/gopath/src/code.google.com/p/go.benchmarks/driver/driver_windows.go:122 +0x5b2

[DanielMorsing]

Comment 1:

Looks like the overlapped variable at driver_windows.go:97 is being filled with a bogus
pointer.

[dvyukov]

Comment 2:

The variable is converted ti pid later:
pid := int(uintptr(unsafe.Pointer(o)))
Also now I am puzzled with this code:
        var o *syscall.Overlapped
        for {
            err := syscall.GetQueuedCompletionStatus(iocp, &code, &key, &o, syscall.INFINITE)
Why do we pass **syscall.Overlapped to GQCS? Shouldn't it be *syscall.Overlapped?

[DanielMorsing]

Comment 3:

http://msdn.microsoft.com/en-gb/library/windows/desktop/aa364986(v=vs.85).aspx suggests
that it's meant as a pointer reference.

[dvyukov]

Comment 4:

Ah, right, it's L*P*OVERLAPPED

[DanielMorsing]

Comment 5:

This should do it. right?
There's still the unfortunate situation of GQCS being able to write both a pointer and a
value to memory. Could/should we change the overlapped arg to be *uintptr?
diff -r 17ba03832124 driver/driver_windows.go
--- a/driver/driver_windows.go  Sat Sep 13 09:10:35 2014 -0700
+++ b/driver/driver_windows.go  Thu Oct 02 13:47:15 2014 +0100
@@ -94,9 +94,10 @@
    // Read Job notifications about new "child" processes and collect them in childProcesses.
    go func() {
        var code, key uint32
-       var o *syscall.Overlapped
+       var o uintptr
+       oa := (**syscall.Overlapped)(unsafe.Pointer(&o))
        for {
-           err := syscall.GetQueuedCompletionStatus(iocp, &code, &key, &o, syscall.INFINITE)
+           err := syscall.GetQueuedCompletionStatus(iocp, &code, &key, oa, syscall.INFINITE)
            if err != nil {
                log.Printf("GetQueuedCompletionStatus failed: %v", err)
                return
@@ -105,7 +106,7 @@
                panic("Invalid GetQueuedCompletionStatus key parameter")
            }
            if code == JOB_OBJECT_MSG_NEW_PROCESS {
-               pid := int(uintptr(unsafe.Pointer(o)))
+               pid := int(o)
                if pid == syscall.Getpid() {
                    continue
                }

[randall77]

Comment 6:

Making "o" a uintptr sounds right.  All we ever do to it is compare it to syscall.Getpid.

[alexbrainman]

Comment 7:

Sounds good to me. Daniel, do you want to send your change for review? Or I can do it
myself, if you like. I won't be able to test it here properly anyway.
Alex

[gopherbot]

Comment 8:

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

[dvyukov]

Comment 9:

This issue was closed by revision c1aee9f08d22.

Status changed to Fixed.