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

cmd/cgo: []byte argument has Go pointer to Go pointer #28606

Open
crawshaw opened this issue Nov 6, 2018 · 6 comments
Open

cmd/cgo: []byte argument has Go pointer to Go pointer #28606

crawshaw opened this issue Nov 6, 2018 · 6 comments
Milestone

Comments

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Nov 6, 2018

With Go 1.11 and HEAD: go version devel +a2a3dd00c9 Thu Sep 13 09:52:57 2018 +0000 darwin/amd64, the following program:

package main

// void f(void* ptr) {}
import "C"

import (
	"compress/gzip"
	"unsafe"
)

type cgoWriter struct{}

func (cgoWriter) Write(p []byte) (int, error) {
	ptr := unsafe.Pointer(&p[0])
	C.f(ptr)
	return 0, nil
}

func main() {
	w := cgoWriter{}
	gzw := gzip.NewWriter(w)
	gzw.Close() // calls cgoWriter.Write
}

Panics:

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

goroutine 1 [running]:
main.cgoWriter.Write.func1(0xc00015e020)
	/Users/crawshaw/junk.go:15 +0x48
main.cgoWriter.Write(0xc00015e020, 0x1, 0xf8, 0xc0000b1e50, 0x4027ff1, 0x40dad60)
	/Users/crawshaw/junk.go:15 +0x35
compress/flate.(*huffmanBitWriter).write(0xc00015e000, 0xc00015e020, 0x1, 0xf8)
	/Users/crawshaw/go/go/src/compress/flate/huffman_bit_writer.go:136 +0x5f
compress/flate.(*huffmanBitWriter).flush(0xc00015e000)
	/Users/crawshaw/go/go/src/compress/flate/huffman_bit_writer.go:128 +0xb7
compress/flate.(*huffmanBitWriter).writeStoredHeader(0xc00015e000, 0x0, 0x1)
	/Users/crawshaw/go/go/src/compress/flate/huffman_bit_writer.go:412 +0x63
compress/flate.(*compressor).close(0xc0000ba000, 0x0, 0xc0000a62b0)
	/Users/crawshaw/go/go/src/compress/flate/deflate.go:647 +0x83
compress/flate.(*Writer).Close(0xc0000ba000, 0x0, 0x0)
	/Users/crawshaw/go/go/src/compress/flate/deflate.go:729 +0x2d
compress/gzip.(*Writer).Close(0xc0000a6210, 0x419bf30, 0xc0000a6210)
	/Users/crawshaw/go/go/src/compress/gzip/gzip.go:242 +0x6e
main.main()
	/Users/crawshaw/junk.go:22 +0x47

I suspect (but cannot yet show that) this is related to the fact that gzip.Writer is passing a []byte to the Write method made out of an array field of the gzip.Writer struct.

@ianlancetaylor ianlancetaylor changed the title runtime/cgo: []byte argument has Go pointer to Go pointer cmd/cgo: []byte argument has Go pointer to Go pointer Nov 6, 2018
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 6, 2018

cgo isn't clever enough to look at where the arguments are coming from. It should work if you write it as

	C.f(unsafe.Pointer(&p[0]))
@Elbertbiggs360

This comment has been minimized.

Copy link

@Elbertbiggs360 Elbertbiggs360 commented Nov 20, 2019

Hi @ianlancetaylor . Is this something that does not need to be addressed anymore? If so, can we close the issue. If not, I would love to pick it up as a first issue

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 20, 2019

As far as I know, this still happens. The program above still fails in the same way.

If you want to look at this, that would be great. But I want to caution you that I don't think this will be easy to fix. At the moment I don't really see how to fix it at all.

@Elbertbiggs360

This comment has been minimized.

Copy link

@Elbertbiggs360 Elbertbiggs360 commented Nov 20, 2019

Awesome. I'm delighted to investigate it and try some options. Reproducing it first..

@Elbertbiggs360

This comment has been minimized.

Copy link

@Elbertbiggs360 Elbertbiggs360 commented Jan 25, 2020

@ianlancetaylor sorry I have been unavailable for a while due to some unforeseen circumstances.
Up until now,

  • I tested out the code and confirmed behaviour
  • I have gone through the unsafe documentation as well as the gzip documentation.
    I observed a note during pointer conversion that said
Note that both conversions must appear in the same expression, with only the intervening arithmetic between them:

That would explain why this is able to work just fine as you mentioned earlier when the expressions are combined.
What I intend to do within the next 3 days

  • Go through the cgo documentation and implementation.

I would love to also look at how the type conversion is implemented. Can you point me in any direction of where this is? Thanks

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 25, 2020

The relevant code here is cmd/cgo/gcc.go, around the method rewriteCall.

I don't think there is going to be any simple fix here. I think that fixing this will require a significant change to how the cgo tool works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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