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

Go 1.6 - C pointer <-> Go pointer rules change causes crash #10

Open
vitaminwater opened this issue Mar 17, 2016 · 5 comments
Open

Go 1.6 - C pointer <-> Go pointer rules change causes crash #10

vitaminwater opened this issue Mar 17, 2016 · 5 comments

Comments

@vitaminwater
Copy link

The new rules: https://golang.org/cmd/cgo/#hdr-Passing_pointers
causes a crash in convert.go line 150:

_, errno = C.iconv(iconv, &inputAsCCharsPtr, &bytesLeftInCSize, &outputCharsPtr, &bytesLeftOutCSize)

With the trace:

panic: runtime error: cgo argument has Go pointer to Go pointer

goroutine 3 [running]:
panic(0x437c780, 0xc82000a1c0)
        /usr/local/Cellar/go/1.6/libexec/src/runtime/panic.go:464 +0x3e6
github.com/GeertJohan/cgo%2ewchar.convertWcharStringToGoString(0xc8200e4100, 0x12, 0x20, 0x0, 0x0, 0x0, 0x0)
        /Users/stant/Documents/hackerloop/rotonde/src/github.com/GeertJohan/cgo.wchar/convert.go:150 +0x521
github.com/GeertJohan/cgo%2ewchar.WcharStringPtrToGoString(0x53059f0, 0x0, 0x0, 0x0, 0x0)
        /Users/stant/Documents/hackerloop/rotonde/src/github.com/GeertJohan/cgo.wchar/wchar.go:141 +0x91
github.com/GeertJohan/go%2ehid.Enumerate(0x42, 0x0, 0x0, 0x0, 0x0, 0x0)
        /Users/stant/Documents/hackerloop/rotonde/src/github.com/GeertJohan/go.hid/hid.go:205 +0x35f
main.StartHID.func2(0xc820010140, 0xc820010160, 0xc820010180, 0xc82006e1e0)

[...]

There must be other places in the code causing this crash.

@vitaminwater
Copy link
Author

I'm not so familiar with the new cgo rules, but I could reproduce the crash.
As I understood my tests, the main issue here is that we have to pass **C.char, with the *C.char being created with the make function of go. ie. make([]C.char, 0, len(ws)*4), which obviously creates a pointer to a go pointer, which falls in the restriction of the new cgo pointer rules.

What I think of as a solution is replacing all make([]C.char, 0, len(ws)*4) to C.malloc calls:

vitaminwater@4a627dc

Which is a problem when it comes to putting data in inputAsCChars, because *C.char is not indexable.
So I created a putWcharAt in the C preamble, but it feels bad to make so many cgo calls.

Anyway I can't test it right now, I need a HID device first, this afternoon, or tomorrow.

@vitaminwater
Copy link
Author

Still trying to fix it, this fork doesn't crash cgo anymore: https://github.com/vitaminwater/cgo.wchar/

But now the whole iconv thing is broken, output for tests is:


--- FAIL: TestStringConversion (0.00s)
        wchar_test.go:18: Error on conversion. invalid argument
--- FAIL: TestWcharStringConversion (0.00s)
        wchar_test.go:43: Error on conversion. illegal byte sequence
--- FAIL: TestRuneConversion (0.00s)
        wchar_test.go:74: Error on conversion. invalid argument

(last test not displayed hangs forever)

Still lacking knowledge, if you could provide me with some input I might create a good pull request to fix it.

@vitaminwater
Copy link
Author

The first test says invalid argument, when I check the desired conversions in convertGoStringToWcharString I see that the tocode parameter for iconv_open is wchar_t, which is not listed when I do a iconv -l, how is that meant to work ?

@vitaminwater
Copy link
Author

Looks like the wchar_t type does not exist on OSX: http://www.firstobject.com/wchar_t-string-on-linux-osx-windows.htm

@vitaminwater
Copy link
Author

Further readings tend to make me think that cgo.wchar could be avoided thanks to golang's unicode package. Is that right ?

Maybe it would be wise to remove cgo.wchar from go.hid ?

@vitaminwater vitaminwater changed the title Go 1.6 cgo <-> Go pointer rules change causes crash Go 1.6 - C pointer <-> Go pointer rules change causes crash Mar 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant