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

Jump to bottom

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

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

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

oxtoacart opened this issue Feb 14, 2015 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.4.2

Comments

@oxtoacart
Copy link
Contributor

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.
@oxtoacart oxtoacart mentioned this issue Feb 14, 2015
Merged
@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 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 as completed Feb 14, 2015
@minux minux added this to the Go1.4.2 milestone Feb 14, 2015
@oxtoacart
Copy link
Contributor Author

@minux Thanks!

@adg
Copy link
Contributor

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 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
@minux @adg
[release-branch.go1.4] runtime: don't panic when given a callback wit…
3124622 
…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 pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@minux @wheatman
[release-branch.go1.4] runtime: don't panic when given a callback wit…
9b49ab5 
…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 pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@minux @wheatman
[release-branch.go1.4] runtime: don't panic when given a callback wit…
e05b7a2 
…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 pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@minux @wheatman
[release-branch.go1.4] runtime: don't panic when given a callback wit…
39c9770 
…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 pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@minux @wheatman
[release-branch.go1.4] runtime: don't panic when given a callback wit…
92fce5f 
…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 pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@minux @wheatman
[release-branch.go1.4] runtime: don't panic when given a callback wit…
60e8ee0 
…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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4.2
Development

No branches or pull requests
4 participants
@minux @oxtoacart @adg @gopherbot