Thanks for the reproducer. I think I've puzzled out what's going wrong and
it's pretty subtle.

TL;DR: You can work around this by either calling `calloc` or just
allocating `inRows` as Go memory and pinning that as well. The latter will
be safer and faster overall. It's not totally clear to me at this moment
whether just using `calloc` instead should be sufficient. Read on for more
details.

Writing pointers into C memory from Go may invoke a write barrier. The
current write barrier is a hybrid barrier: it writes down the pointers that
are both being written, and those that are being deleted in the write.

In this particular case, we're writing a Go pointer into C memory *from Go*.
If that C memory isn't zeroed *and* it points to some dead Go memory, we
have a problem. That appears to be what's happening here. If I use calloc
instead of malloc to create inRows, the problem goes away. Also, if I
create inRows in Go and pin it, that also works.

I think this is actually documented in the cgo pointer passing rules (
https://pkg.go.dev/cmd/cgo#hdr-Passing_pointers), but we all missed it:

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.


AFAICT, the rest of the page does not say one way or the other whether Go
code writing a Go pointer into C memory is allowed (note that,
counterintuitively, *C.float is a Go pointer, because it may point to Go
memory).

One question that I have to wonder about is whether it's OK to write a Go
pointer into C memory at all and if we should just restrict that like we
did before. The text above already implies that it is not OK, in which case
the code in this thread is invalid and inRows basically must be allocated
on the Go side. However, I think this may also just be an oversight from
when I was updating the documentation for pinning; I think my original
intent was to say that it's OK for Go code to write pinned Go pointers to C
memory.

Anyway, this needs more thought, since allowing Go code to write Go
pointers to C memory (pinned or not) may end up restricting GC
implementations in a way we don't want to. I filed
https://github.com/golang/go/issues/65349 to track this.

On Mon, Jan 29, 2024 at 11:17 AM peterGo <go.peter...@gmail.com> wrote:

> Michael,
>
> The OP appears to have lost interest in debugging. Here's my minimal,
> reproducible example that produces the same fatal errror:
>
> https://go.dev/play/p/flEmSh1euqR (run locally)
>
> If the runtime.GC() statement is uncommented then the fatal error does not
> occur.
>
> The use of this profligate, inefficient algorithm is for debugging
> purposes only. The use of runtime.Pinner is not required. A single
> implicitly pinned C wrapper function argument pointing to contiguous
> underlying slice data arrays would suffice.
>
> Peter
>
>
> On Friday, January 26, 2024 at 12:05:38 PM UTC-5 Michael Knyszek wrote:
>
>> Ignoring more efficient ways to pass memory to C for a moment,
>> superficially I do think your code using Pinner should work. Do you have a
>> full reproducer? It's hard to tell from just looking at your code if your
>> code is the problem, or its just enough to trigger some other cgo issue
>> elsewhere in your codebase. There's a slim chance this is a bug in the
>> Pinner implementation, but that would be surprising to me. The Pinner
>> trivially keeps all pointers passed to it live by just holding onto their
>> references.
>>
>> Also, have you tried running your code with GODEBUG=cgocheck=1 and/or
>> building your code with GOEXPERIMENT=cgocheck2? That might help identify
>> what the issue is.
>> On Friday, January 26, 2024 at 7:05:45 AM UTC-5 Tamás Gulácsi wrote:
>>
>>> To convert a Go slice to C array is easy with unsafe.Slice.
>>>
>>> The problem is that the multi-dimensional C array is an array of
>>> pointers (*(*float64))
>>> which cannot travel between the C/Go barrier.
>>>
>>> In your example, you flatten your slice, and recreate the pointers in
>>> that one big slice.
>>> You could go on with that example, but from the Go side:
>>>
>>> 1. have a "flat" []float64 wihich is N*M
>>> 2. have the [][]float64 as subslices of that one big flattened
>>> []float64: flat[i*M:i*(M+1)]
>>> 3. send that flat slice to the C side with unsafe.Slice
>>> 4. have the C side implement the same subslicing: create a []*float64
>>> array and have each point to the corredt &flat[i*M].
>>> 5. profit
>>>
>>>
>>> Denis a következőt írta (2024. január 26., péntek, 1:18:59 UTC+1):
>>>
>>>> I am trying to pass 2d array from Go to some C function void foo(in
>>>> **float, out *double). Since I want to have wrapper for this C
>>>> function, I'd like that Go function has definition like func
>>>> FooWrapper([][]float32) []float64. The easiest but not efficient
>>>> implementation is allocating all memory through C that listed below:
>>>>
>>>> func FooWrapper(values [][]float32) []float64 {
>>>>     totalObj := len(values)
>>>>     totalParams := len(values[0])
>>>>     results := make([]float64, totalObj)
>>>>
>>>>     ptrArrLength := uintptr(totalObj) * cPointerSize
>>>>     paramArrLength := uintptr(totalParams) * cFloatSize
>>>>
>>>>     ptr := C.malloc(C.size_t(ptrArrLength +
>>>> paramArrLength*uintptr(totalObj)))
>>>>     defer C.free(ptr)
>>>>
>>>>     ptrSlice := unsafe.Slice((**C.float)(ptr), totalObj)
>>>>     for i, obj := range values {
>>>>         paramArrPtr := (*C.float)(unsafe.Add(ptr,
>>>> ptrArrLength+uintptr(i)*paramArrLength))
>>>>         ptrSlice[i] = paramArrPtr
>>>>
>>>>         paramSlice := unsafe.Slice(paramArrPtr, totalParams)
>>>>         for j, param := range obj {
>>>>             paramSlice[j] = (C.float)(param)
>>>>         }
>>>>     }
>>>>
>>>>     C.foo((**C.float)(ptr), (*C.double)(&results[0]))
>>>>
>>>>     return results
>>>> }
>>>>
>>>> Is that safe implementation? Can I pass pointer of result data? As far
>>>> as I know, this pointer will be pinned because it passed to C function.
>>>>
>>>> But I want to allocate less memory just reusing Go memory, I've learned
>>>> about runtime.Pinner that make pointer pinned until
>>>> runtime.Pinner.Unpin() invocation. I tried to write another
>>>> implementation using pinner:
>>>>
>>>> func FooWrapper(values [][]float32) []float64 {
>>>>     length := len(values)
>>>>     results := make([]float64, length)
>>>>
>>>>     pinner := runtime.Pinner{}
>>>>     defer pinner.Unpin()
>>>>
>>>>     arr := (**C.float)(C.malloc(C.size_t(uintptr(length) *
>>>> cPointerSize)))
>>>>     defer C.free(unsafe.Pointer(arr))
>>>>     slice := unsafe.Slice(arr, length)
>>>>     for i, v := range values {
>>>>         pinner.Pin(&v[0])
>>>>         slice[i] = (*C.float)(&v[0])
>>>>     }
>>>>
>>>>     C.foo(arr, (*C.double)(&results[0]))
>>>>
>>>>     return results
>>>> }
>>>>
>>>> But, unfortunately, this code doesn't work
>>>>
>>>> runtime: pointer 0xc016ecbfc0 to unused region of span
>>>> span.base()=0xc016eca000 span.limit=0xc016ecbfa0 span.state=1
>>>> fatal error: found bad pointer in Go heap (incorrect use of unsafe or
>>>> cgo?)
>>>>
>>>> Do I use runtime.Pinner wrong (as far as I know, I can pin slice data)?
>>>> Or there is another error in this code. Are there some implementations for
>>>> passing 3d (4d and so on) array to C function except for allocatiing and
>>>> copying all data to C memory?
>>>>
>>> --
> You received this message because you are subscribed to a topic in the
> Google Groups "golang-nuts" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/golang-nuts/_7bUuHX0ca4/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> golang-nuts+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/golang-nuts/582e4c81-28f2-40f5-a083-259a897bb45an%40googlegroups.com
> <https://groups.google.com/d/msgid/golang-nuts/582e4c81-28f2-40f5-a083-259a897bb45an%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAFza%2Bu8YcyY4NBmZxfo93ecY70Oy3wd_sVffLSsrpnq2hOV%2B8g%40mail.gmail.com.

Reply via email to