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

handle allocations for problematic structs to avoid sharing pointers-to-pointers with C (from Go) #82

Merged
merged 8 commits into from
May 5, 2020

Conversation

laser
Copy link
Contributor

@laser laser commented May 5, 2020

For additional background, read this thread.

Why does this PR exist?

When running software that consumed filecoin-ffi, setting GODEBUG=cgocheck=2 option produced the following error:

write of Go pointer 0xc000027b48 to non-Go memory 0x8900000
fatal error: Go pointer stored into non-Go memory

The cause of this error is that c-for-go generated code which mapped a slice of Go structs to a dynamic array of their C equivalents, but when those Go structs had fields which were string or []byte, a pointer to the original Go slice would be shared with C (instead of copying the nested value into the C heap). I suspect that this was by-design.

What's in this PR?

To appease the gods, I have written custom allocating methods for the two structs which cause the reported issue. I have also rewritten the fil_verify_hash function to accept a flattened byte array, which prevents the double-pointer error.

@laser laser merged commit 0c54e22 into master May 5, 2020
@laser laser deleted the feat/cgocheck-2-panic branch May 5, 2020 19:37
gracenoah pushed a commit to gracenoah/filecoin-ffi that referenced this pull request Dec 20, 2020
…to-pointers with C (from Go) (filecoin-project#82)

* alternative verify post

* manually allocate proxy object to prevent sharing Go heap with C

* Revert "alternative verify post"

This reverts commit 841ff78.

* manually allocate proxy object to prevent sharing Go heap with C

* pass flattened messages to fil_hash_verify to prevent pointer-to-pointer issues with CGO

* rename symbols within custom allocate methods

* run go test with cgocheck=2

* delete temporary branches from circleci config
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

Successfully merging this pull request may close these issues.

2 participants