Skip to content

fix(arrow/cdata): Avoid calling unsafe.Slice on zero-length pointers#513

Merged
zeroshade merged 3 commits intoapache:mainfrom
orlp:empty-slice
Sep 22, 2025
Merged

fix(arrow/cdata): Avoid calling unsafe.Slice on zero-length pointers#513
zeroshade merged 3 commits intoapache:mainfrom
orlp:empty-slice

Conversation

@orlp
Copy link
Contributor

@orlp orlp commented Sep 22, 2025

Rationale for this change

Slices from FFI may have an arbitrary pointer when the length is zero, but this is not allowed in Go, where the pointer must always be valid. I believe this fixes #28.

What changes are included in this PR?

I changed the instances of unsafe.Slice in arrow/cdata I could find to be robust when used with length zero.

Are these changes tested?

No, I don't have a Go setup at all.

Are there any user-facing changes?

No.

@orlp orlp requested a review from zeroshade as a code owner September 22, 2025 17:54
@zeroshade zeroshade changed the title Avoid calling unsafe.Slice on zero-length pointers fix(arrow/cdata): Avoid calling unsafe.Slice on zero-length pointers Sep 22, 2025
@zeroshade
Copy link
Member

I'm gonna put together a unit test that hits the issue and see if this fixes it, if so I'll push the unit test up to this

@zeroshade
Copy link
Member

thanks for putting this together @orlp. You beat me to putting up the change 😄 The test was able to confirm that the 0x1 got through before the changes, and afterwards it is a proper Go empty slice.

@zeroshade zeroshade merged commit 41441ea into apache:main Sep 22, 2025
16 checks passed
@orlp
Copy link
Contributor Author

orlp commented Sep 22, 2025

@zeroshade Actually, a friend just told me that even raw pointers in Go always need to be valid, not just base addresses of slices. If that's true then this fix is insufficient.

@zeroshade
Copy link
Member

That is technically correct, though the checks don't look in C/C++ memory. Essentially, as long as we don't attempt to store ArrowArray.buffers[2] into a Go variable when it's not a valid pointer (which I believe we successfully avoid now) we should be safe because you only find the bad pointer by first dereferencing the void** in ArrowArray.buffers.

If we do manage to hit this still, then we can look into adding a check in the trampoline function we have

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.

[Go] Fatal Error in pqarrow.writeDenseArrow : invalid pointer

2 participants