Skip to content

Avoid cgo bug by use C.calloc to allocate memory in C instead of C.ma…#61

Merged
valyala merged 1 commit intovalyala:masterfrom
waldoweng:master
May 20, 2024
Merged

Avoid cgo bug by use C.calloc to allocate memory in C instead of C.ma…#61
valyala merged 1 commit intovalyala:masterfrom
waldoweng:master

Conversation

@waldoweng
Copy link
Copy Markdown
Contributor

@waldoweng waldoweng commented Apr 25, 2024

…lloc,

See https://pkg.go.dev/cmd/cgo:

Note: the current implementation has a bug. While Go code is permitted to write nil or a C pointer (but not a Go pointer) to C memory, the current implementation may sometimes cause a runtime error if the contents of the C memory appear to be a Go pointer. Therefore, avoid passing uninitialized C memory to Go code if the Go code is going to store pointer values in it. Zero out the memory in C before passing it to Go.

I also find similar approach use by Dgraph according to this blog post:
https://dgraph.io/blog/post/manual-memory-management-golang-jemalloc/

So, instead of using malloc, we use its slightly more expensive sibling, calloc. calloc works the same way as malloc, except it zeroes out the memory before returning it to the caller.

…lloc,

See https://pkg.go.dev/cmd/cgo:

Note: the current implementation has a bug. While Go code is permitted to write nil or a C pointer (but not a Go pointer) to C memory, the current implementation may sometimes cause a runtime error if the contents of the C memory appear to be a Go pointer. Therefore, avoid passing uninitialized C memory to Go code if the Go code is going to store pointer values in it. Zero out the memory in C before passing it to Go.
@f41gh7
Copy link
Copy Markdown
Collaborator

f41gh7 commented May 16, 2024

Hello, do you have any examples with panic related to this lib?

As far as I understand current issue with CGO. It's only possible to get a panic, if program makes allocation of uninitialized objects via malloc and shadows it with GO allocations. Usual example for it is slice resizing via append.

@f41gh7
Copy link
Copy Markdown
Collaborator

f41gh7 commented May 16, 2024

Related issue golang/go#19928

@valyala valyala merged commit 8e7a79f into valyala:master May 20, 2024
@valyala
Copy link
Copy Markdown
Owner

valyala commented May 20, 2024

@waldoweng , thanks for the fix!

nilz3000 pushed a commit to nilz3000/gozstd that referenced this pull request Jul 30, 2024
…lloc, (valyala#61)

See https://pkg.go.dev/cmd/cgo:

Note: the current implementation has a bug. While Go code is permitted to write nil or a C pointer (but not a Go pointer) to C memory, the current implementation may sometimes cause a runtime error if the contents of the C memory appear to be a Go pointer. Therefore, avoid passing uninitialized C memory to Go code if the Go code is going to store pointer values in it. Zero out the memory in C before passing it to Go.
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.

3 participants