Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall: NewCallback panics when given a callback function with no input parameters #9871

Closed
oxtoacart opened this issue Feb 14, 2015 · 4 comments
Milestone

Comments

@oxtoacart
Copy link
Contributor

@oxtoacart oxtoacart commented Feb 14, 2015

Go 1.4.1 on Windows XP 32-bit (386)

The below program panics:

package main

import (
        "log"
        "syscall"
)

func main() {
        cb := syscall.NewCallback(callback)
        log.Println(cb)
}

func callback() uintptr {
        return 0
}
$ go run callback.go
panic: runtime error: index out of range

goroutine 1 [running]:
syscall.NewCallback(0x499ba0, 0x4dfb68, 0x434503)
        c:/go/src/syscall/syscall_windows.go:115 +0x35
main.main()
        c:/Documents and Settings/Administrator/gocode/src/callback.go:9 +0x3a

goroutine 3 [runnable]:
runtime.bgsweep()
        c:/go/src/runtime/mgc0.go:82
runtime.goexit()
        c:/go/src/runtime/asm_386.s:2287 +0x1

goroutine 4 [runnable]:
runtime.runfinq()
        c:/go/src/runtime/malloc.go:712
runtime.goexit()
        c:/go/src/runtime/asm_386.s:2287 +0x1
exit status 2

However this program does not panic:

package main

import (
        "log"
        "syscall"
)

func main() {
        cb := syscall.NewCallback(callback)
        log.Println(cb)
}

func callback(ignored uintptr) uintptr {
        return 0
}

It seems that syscall.NewCallback requires the callback function to have at least one input parameter. Note that in Go 1.3.3 it did not require the callback function to have at least one input parameter, so to avoid breaking existing code, it should probably continue not to require it. However, if it does continue to require an input parameter, it should at least return a human-readable error and the documentation for the function should make that clear.

Note also that I get the same result with syscall.NewCallbackCDecl.

@minux minux changed the title On Windows, syscall.NewCallback panics when given a callback function with no input parameters syscall: NewCallback panics when given a callback function with no input parameters Feb 14, 2015
@minux
Copy link
Member

@minux minux commented Feb 14, 2015

It has been fixed on tip, and the following patch should fix it in 1.4.1.

--- src/runtime/syscall_windows.go 2015-02-14 00:09:23.834294000 -0500
+++ src/runtime/syscall_windows.go  2015-02-14 00:09:50.474651894 -0500
@@ -54,11 +54,13 @@ func compileCallback(fn eface, cleanstac
                panic("compilecallback: output parameter size is wrong")
        }
        argsize := uintptr(0)
-       for _, t := range (*[1024](*_type))(unsafe.Pointer(&ft.in[0]))[:len(ft.in)] {
-               if (*t).size > uintptrSize {
-                       panic("compilecallback: input parameter size is wrong")
+       if len(ft.in) > 0 {
+               for _, t := range (*[1024](*_type))(unsafe.Pointer(&ft.in[0]))[:len(ft.in)] {
+                       if (*t).size > uintptrSize {
+                               panic("compilecallback: input parameter size is wrong")
+                       }
+                       argsize += uintptrSize
                }
-               argsize += uintptrSize
        }

        lock(&cbs.lock)

I'm not sure if we want to fix it for 1.4.2 or not. Tentatively labeled Go1.4.2.

@minux minux closed this Feb 14, 2015
@minux minux added the repo-main label Feb 14, 2015
@minux minux added this to the Go1.4.2 milestone Feb 14, 2015
@oxtoacart
Copy link
Contributor Author

@oxtoacart oxtoacart commented Feb 14, 2015

@minux Thanks!

@adg
Copy link
Contributor

@adg adg commented Feb 17, 2015

@minux your patch does not apply cleanly to release-branch.go1.4. Did I miss a change go by?

@minux
Copy link
Member

@minux minux commented Feb 17, 2015

I will send a CL against release-branch.go1.4.

minux added a commit that referenced this issue Feb 17, 2015
…h no input params on windows

Fixes #9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
…h no input params on windows

Fixes golang#9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
…h no input params on windows

Fixes golang#9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
…h no input params on windows

Fixes golang#9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
…h no input params on windows

Fixes golang#9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
…h no input params on windows

Fixes golang#9871 for Go 1.4.

Change-Id: I550a5bdb29e9a872652e0dd468a434227d7d9502
Reviewed-on: https://go-review.googlesource.com/4937
Run-TryBot: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.