diff --git a/go/adbc/pkg/_tmpl/driver.go.tmpl b/go/adbc/pkg/_tmpl/driver.go.tmpl index d68170f2db..05b2b11a3d 100644 --- a/go/adbc/pkg/_tmpl/driver.go.tmpl +++ b/go/adbc/pkg/_tmpl/driver.go.tmpl @@ -230,22 +230,22 @@ func printLoggingHelp() { } -// Allocate a new cgo.Handle and store its address in a heap-allocated -// uintptr_t. Experimentally, this was found to be necessary, else -// something (the Go runtime?) would corrupt (garbage-collect?) the -// handle. +// cgo.Handle is a uintptr integer (not a pointer). Packing it directly into +// a void* field is safe: the CGO checker only rejects Go heap pointers, and +// handle values (small non-zero integers from a global counter) never alias +// Go-allocated memory. The GC does not scan C-managed memory, so it will +// never misinterpret the stored integer as a live pointer. No C allocation +// is needed — the handle value itself fits in the pointer-sized field. func createHandle(hndl cgo.Handle) unsafe.Pointer { - // uintptr_t* hptr = malloc(sizeof(uintptr_t)); - hptr := (*C.uintptr_t)(C.calloc(C.sizeof_uintptr_t, C.size_t(1))) - // *hptr = (uintptr)hndl; - *hptr = C.uintptr_t(uintptr(hndl)) - return unsafe.Pointer(hptr) + return unsafe.Pointer(uintptr(hndl)) +} + +func handleFromPtr(ptr unsafe.Pointer) cgo.Handle { + return cgo.Handle(uintptr(ptr)) } func getFromHandle[T any](ptr unsafe.Pointer) *T { - // uintptr_t* hptr = (uintptr_t*)ptr; - hptr := (*C.uintptr_t)(ptr) - return cgo.Handle((uintptr)(*hptr)).Value().(*T) + return handleFromPtr(ptr).Value().(*T) } func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { @@ -253,7 +253,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC if lenWithTerminator <= *length { sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) copy(sink, val) - sink[lenWithTerminator] = 0 + sink[len(val)] = 0 } *length = lenWithTerminator return C.ADBC_STATUS_OK @@ -417,17 +417,16 @@ func {{.Prefix}}ArrayStreamRelease(stream *C.struct_ArrowArrayStream) { if stream == nil || stream.release != (*[0]byte)(C.{{.Prefix}}ArrayStreamRelease) || stream.private_data == nil { return } - h := (*(*cgo.Handle)(stream.private_data)) + h := handleFromPtr(stream.private_data) + stream.private_data = nil cStream := h.Value().(*cArrayStream) + h.Delete() cStream.rdr.Release() if cStream.adbcErr != nil { C.{{.Prefix}}errRelease(cStream.adbcErr) C.free(unsafe.Pointer(cStream.adbcErr)) } - C.free(unsafe.Pointer(stream.private_data)) - stream.private_data = nil - h.Delete() runtime.GC() } @@ -614,19 +613,16 @@ func {{.Prefix}}DatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcErr if !checkDBAlloc(db, err, "AdbcDatabaseRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(db.private_data)) + h := handleFromPtr(db.private_data) + db.private_data = nil cdb := h.Value().(*cDatabase) + h.Delete() if cdb.db != nil { cdb.db.Close() cdb.db = nil } cdb.opts = nil - if db.private_data != nil { - C.free(unsafe.Pointer(db.private_data)) - db.private_data = nil - } - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1029,15 +1025,14 @@ func {{.Prefix}}ConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_A if !checkConnAlloc(cnxn, err, "AdbcConnectionRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(cnxn.private_data)) + h := handleFromPtr(cnxn.private_data) + cnxn.private_data = nil conn := h.Value().(*cConn) + h.Delete() defer func() { conn.cancelContext() conn.cnxn = nil - C.free(cnxn.private_data) - cnxn.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1479,15 +1474,14 @@ func {{.Prefix}}StatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_Adb if !checkStmtAlloc(stmt, err, "AdbcStatementRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(stmt.private_data)) + h := handleFromPtr(stmt.private_data) + stmt.private_data = nil st := h.Value().(*cStmt) + h.Delete() defer func() { st.cancelContext() st.stmt = nil - C.free(stmt.private_data) - stmt.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually diff --git a/go/adbc/pkg/flightsql/driver.go b/go/adbc/pkg/flightsql/driver.go index defe870ffa..3cecf4f79a 100644 --- a/go/adbc/pkg/flightsql/driver.go +++ b/go/adbc/pkg/flightsql/driver.go @@ -233,22 +233,22 @@ func printLoggingHelp() { fmt.Fprintf(os.Stderr, "FlightSQL: to enable logging, set %s to 'debug', 'info', 'warn', or 'error'", logLevelEnvVar) } -// Allocate a new cgo.Handle and store its address in a heap-allocated -// uintptr_t. Experimentally, this was found to be necessary, else -// something (the Go runtime?) would corrupt (garbage-collect?) the -// handle. +// cgo.Handle is a uintptr integer (not a pointer). Packing it directly into +// a void* field is safe: the CGO checker only rejects Go heap pointers, and +// handle values (small non-zero integers from a global counter) never alias +// Go-allocated memory. The GC does not scan C-managed memory, so it will +// never misinterpret the stored integer as a live pointer. No C allocation +// is needed — the handle value itself fits in the pointer-sized field. func createHandle(hndl cgo.Handle) unsafe.Pointer { - // uintptr_t* hptr = malloc(sizeof(uintptr_t)); - hptr := (*C.uintptr_t)(C.calloc(C.sizeof_uintptr_t, C.size_t(1))) - // *hptr = (uintptr)hndl; - *hptr = C.uintptr_t(uintptr(hndl)) - return unsafe.Pointer(hptr) + return unsafe.Pointer(uintptr(hndl)) +} + +func handleFromPtr(ptr unsafe.Pointer) cgo.Handle { + return cgo.Handle(uintptr(ptr)) } func getFromHandle[T any](ptr unsafe.Pointer) *T { - // uintptr_t* hptr = (uintptr_t*)ptr; - hptr := (*C.uintptr_t)(ptr) - return cgo.Handle((uintptr)(*hptr)).Value().(*T) + return handleFromPtr(ptr).Value().(*T) } func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { @@ -256,7 +256,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC if lenWithTerminator <= *length { sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) copy(sink, val) - sink[lenWithTerminator] = 0 + sink[len(val)] = 0 } *length = lenWithTerminator return C.ADBC_STATUS_OK @@ -420,17 +420,16 @@ func FlightSQLArrayStreamRelease(stream *C.struct_ArrowArrayStream) { if stream == nil || stream.release != (*[0]byte)(C.FlightSQLArrayStreamRelease) || stream.private_data == nil { return } - h := (*(*cgo.Handle)(stream.private_data)) + h := handleFromPtr(stream.private_data) + stream.private_data = nil cStream := h.Value().(*cArrayStream) + h.Delete() cStream.rdr.Release() if cStream.adbcErr != nil { C.FlightSQLerrRelease(cStream.adbcErr) C.free(unsafe.Pointer(cStream.adbcErr)) } - C.free(unsafe.Pointer(stream.private_data)) - stream.private_data = nil - h.Delete() runtime.GC() } @@ -617,19 +616,16 @@ func FlightSQLDatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcError if !checkDBAlloc(db, err, "AdbcDatabaseRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(db.private_data)) + h := handleFromPtr(db.private_data) + db.private_data = nil cdb := h.Value().(*cDatabase) + h.Delete() if cdb.db != nil { cdb.db.Close() cdb.db = nil } cdb.opts = nil - if db.private_data != nil { - C.free(unsafe.Pointer(db.private_data)) - db.private_data = nil - } - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1032,15 +1028,14 @@ func FlightSQLConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_Adb if !checkConnAlloc(cnxn, err, "AdbcConnectionRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(cnxn.private_data)) + h := handleFromPtr(cnxn.private_data) + cnxn.private_data = nil conn := h.Value().(*cConn) + h.Delete() defer func() { conn.cancelContext() conn.cnxn = nil - C.free(cnxn.private_data) - cnxn.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1482,15 +1477,14 @@ func FlightSQLStatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_AdbcE if !checkStmtAlloc(stmt, err, "AdbcStatementRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(stmt.private_data)) + h := handleFromPtr(stmt.private_data) + stmt.private_data = nil st := h.Value().(*cStmt) + h.Delete() defer func() { st.cancelContext() st.stmt = nil - C.free(stmt.private_data) - stmt.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually diff --git a/go/adbc/pkg/panicdummy/driver.go b/go/adbc/pkg/panicdummy/driver.go index f7b55eda1e..07bbc4ac62 100644 --- a/go/adbc/pkg/panicdummy/driver.go +++ b/go/adbc/pkg/panicdummy/driver.go @@ -233,22 +233,22 @@ func printLoggingHelp() { fmt.Fprintf(os.Stderr, "PanicDummy: to enable logging, set %s to 'debug', 'info', 'warn', or 'error'", logLevelEnvVar) } -// Allocate a new cgo.Handle and store its address in a heap-allocated -// uintptr_t. Experimentally, this was found to be necessary, else -// something (the Go runtime?) would corrupt (garbage-collect?) the -// handle. +// cgo.Handle is a uintptr integer (not a pointer). Packing it directly into +// a void* field is safe: the CGO checker only rejects Go heap pointers, and +// handle values (small non-zero integers from a global counter) never alias +// Go-allocated memory. The GC does not scan C-managed memory, so it will +// never misinterpret the stored integer as a live pointer. No C allocation +// is needed — the handle value itself fits in the pointer-sized field. func createHandle(hndl cgo.Handle) unsafe.Pointer { - // uintptr_t* hptr = malloc(sizeof(uintptr_t)); - hptr := (*C.uintptr_t)(C.calloc(C.sizeof_uintptr_t, C.size_t(1))) - // *hptr = (uintptr)hndl; - *hptr = C.uintptr_t(uintptr(hndl)) - return unsafe.Pointer(hptr) + return unsafe.Pointer(uintptr(hndl)) +} + +func handleFromPtr(ptr unsafe.Pointer) cgo.Handle { + return cgo.Handle(uintptr(ptr)) } func getFromHandle[T any](ptr unsafe.Pointer) *T { - // uintptr_t* hptr = (uintptr_t*)ptr; - hptr := (*C.uintptr_t)(ptr) - return cgo.Handle((uintptr)(*hptr)).Value().(*T) + return handleFromPtr(ptr).Value().(*T) } func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { @@ -256,7 +256,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC if lenWithTerminator <= *length { sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) copy(sink, val) - sink[lenWithTerminator] = 0 + sink[len(val)] = 0 } *length = lenWithTerminator return C.ADBC_STATUS_OK @@ -420,17 +420,16 @@ func PanicDummyArrayStreamRelease(stream *C.struct_ArrowArrayStream) { if stream == nil || stream.release != (*[0]byte)(C.PanicDummyArrayStreamRelease) || stream.private_data == nil { return } - h := (*(*cgo.Handle)(stream.private_data)) + h := handleFromPtr(stream.private_data) + stream.private_data = nil cStream := h.Value().(*cArrayStream) + h.Delete() cStream.rdr.Release() if cStream.adbcErr != nil { C.PanicDummyerrRelease(cStream.adbcErr) C.free(unsafe.Pointer(cStream.adbcErr)) } - C.free(unsafe.Pointer(stream.private_data)) - stream.private_data = nil - h.Delete() runtime.GC() } @@ -617,19 +616,16 @@ func PanicDummyDatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcErro if !checkDBAlloc(db, err, "AdbcDatabaseRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(db.private_data)) + h := handleFromPtr(db.private_data) + db.private_data = nil cdb := h.Value().(*cDatabase) + h.Delete() if cdb.db != nil { cdb.db.Close() cdb.db = nil } cdb.opts = nil - if db.private_data != nil { - C.free(unsafe.Pointer(db.private_data)) - db.private_data = nil - } - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1032,15 +1028,14 @@ func PanicDummyConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_Ad if !checkConnAlloc(cnxn, err, "AdbcConnectionRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(cnxn.private_data)) + h := handleFromPtr(cnxn.private_data) + cnxn.private_data = nil conn := h.Value().(*cConn) + h.Delete() defer func() { conn.cancelContext() conn.cnxn = nil - C.free(cnxn.private_data) - cnxn.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1482,15 +1477,14 @@ func PanicDummyStatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_Adbc if !checkStmtAlloc(stmt, err, "AdbcStatementRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(stmt.private_data)) + h := handleFromPtr(stmt.private_data) + stmt.private_data = nil st := h.Value().(*cStmt) + h.Delete() defer func() { st.cancelContext() st.stmt = nil - C.free(stmt.private_data) - stmt.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually diff --git a/go/adbc/pkg/snowflake/driver.go b/go/adbc/pkg/snowflake/driver.go index 30b9fe04f6..6168587512 100644 --- a/go/adbc/pkg/snowflake/driver.go +++ b/go/adbc/pkg/snowflake/driver.go @@ -233,22 +233,22 @@ func printLoggingHelp() { fmt.Fprintf(os.Stderr, "Snowflake: to enable logging, set %s to 'debug', 'info', 'warn', or 'error'", logLevelEnvVar) } -// Allocate a new cgo.Handle and store its address in a heap-allocated -// uintptr_t. Experimentally, this was found to be necessary, else -// something (the Go runtime?) would corrupt (garbage-collect?) the -// handle. +// cgo.Handle is a uintptr integer (not a pointer). Packing it directly into +// a void* field is safe: the CGO checker only rejects Go heap pointers, and +// handle values (small non-zero integers from a global counter) never alias +// Go-allocated memory. The GC does not scan C-managed memory, so it will +// never misinterpret the stored integer as a live pointer. No C allocation +// is needed — the handle value itself fits in the pointer-sized field. func createHandle(hndl cgo.Handle) unsafe.Pointer { - // uintptr_t* hptr = malloc(sizeof(uintptr_t)); - hptr := (*C.uintptr_t)(C.calloc(C.sizeof_uintptr_t, C.size_t(1))) - // *hptr = (uintptr)hndl; - *hptr = C.uintptr_t(uintptr(hndl)) - return unsafe.Pointer(hptr) + return unsafe.Pointer(uintptr(hndl)) +} + +func handleFromPtr(ptr unsafe.Pointer) cgo.Handle { + return cgo.Handle(uintptr(ptr)) } func getFromHandle[T any](ptr unsafe.Pointer) *T { - // uintptr_t* hptr = (uintptr_t*)ptr; - hptr := (*C.uintptr_t)(ptr) - return cgo.Handle((uintptr)(*hptr)).Value().(*T) + return handleFromPtr(ptr).Value().(*T) } func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { @@ -256,7 +256,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC if lenWithTerminator <= *length { sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) copy(sink, val) - sink[lenWithTerminator] = 0 + sink[len(val)] = 0 } *length = lenWithTerminator return C.ADBC_STATUS_OK @@ -420,17 +420,16 @@ func SnowflakeArrayStreamRelease(stream *C.struct_ArrowArrayStream) { if stream == nil || stream.release != (*[0]byte)(C.SnowflakeArrayStreamRelease) || stream.private_data == nil { return } - h := (*(*cgo.Handle)(stream.private_data)) + h := handleFromPtr(stream.private_data) + stream.private_data = nil cStream := h.Value().(*cArrayStream) + h.Delete() cStream.rdr.Release() if cStream.adbcErr != nil { C.SnowflakeerrRelease(cStream.adbcErr) C.free(unsafe.Pointer(cStream.adbcErr)) } - C.free(unsafe.Pointer(stream.private_data)) - stream.private_data = nil - h.Delete() runtime.GC() } @@ -617,19 +616,16 @@ func SnowflakeDatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcError if !checkDBAlloc(db, err, "AdbcDatabaseRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(db.private_data)) + h := handleFromPtr(db.private_data) + db.private_data = nil cdb := h.Value().(*cDatabase) + h.Delete() if cdb.db != nil { cdb.db.Close() cdb.db = nil } cdb.opts = nil - if db.private_data != nil { - C.free(unsafe.Pointer(db.private_data)) - db.private_data = nil - } - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1032,15 +1028,14 @@ func SnowflakeConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_Adb if !checkConnAlloc(cnxn, err, "AdbcConnectionRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(cnxn.private_data)) + h := handleFromPtr(cnxn.private_data) + cnxn.private_data = nil conn := h.Value().(*cConn) + h.Delete() defer func() { conn.cancelContext() conn.cnxn = nil - C.free(cnxn.private_data) - cnxn.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually @@ -1482,15 +1477,14 @@ func SnowflakeStatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_AdbcE if !checkStmtAlloc(stmt, err, "AdbcStatementRelease") { return C.ADBC_STATUS_INVALID_STATE } - h := (*(*cgo.Handle)(stmt.private_data)) + h := handleFromPtr(stmt.private_data) + stmt.private_data = nil st := h.Value().(*cStmt) + h.Delete() defer func() { st.cancelContext() st.stmt = nil - C.free(stmt.private_data) - stmt.private_data = nil - h.Delete() // manually trigger GC for two reasons: // 1. ASAN expects the release callback to be called before // the process ends, but GC is not deterministic. So by manually