From 9cf4bf0127ae99e28fe0090650125a26c2717c41 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Sun, 31 May 2026 11:37:05 +0200 Subject: [PATCH] feat(ffi): native typed struct returns for the C ABI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `{.ffi.}` proc that returns a registered struct now delivers it natively instead of CBOR-encoding it. The FFI-thread handler builds the return's `Pod` mirror on the heap (`nimToPod`) and stashes it on the request; the callback receives it as a typed `const *` (msg = pointer, len = sizeof), and handleRes deep-frees it the instant the callback returns — callback-lifetime ownership, the caller frees nothing. Mechanics: FFIThreadRequest gains respPod/respPodLen/respPodFree fields that handleRes honors ahead of the byte payload; the macro emits a per-proc cdecl freer (`freePod` + `ffiCFree`) for the response POD. String and seq[byte] returns still travel as raw bytes; the CBOR path (`_cbor`) is a separate handler and is unchanged. The C header documents the new return shape. Validated end-to-end from C (EchoResponse, ComplexResponse with nested seq/option graphs) including under ASAN — no UAF or double-free. Co-Authored-By: Claude Opus 4.8 --- ffi/codegen/c.nim | 13 ++++--- ffi/ffi_thread_request.nim | 27 ++++++++++++++ ffi/internal/ffi_macro.nim | 72 ++++++++++++++++++++++++++++++-------- 3 files changed, 94 insertions(+), 18 deletions(-) diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index b8fcc48..f4010c8 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -95,10 +95,15 @@ const HeaderPrelude = """ // Generated by nim-ffi C codegen. Do not edit by hand. // // Native (zero-serialization) C ABI. Each call delivers its result to the -// callback: on RET_OK, (msg, len) is the raw return value (for string-returning -// procs, the string bytes — not NUL-terminated; use len); on RET_ERR, (msg, len) -// is the raw error text. A `_cbor` variant of each proc also exists for -// generic/cross-language callers that prefer a CBOR request/response. +// callback. On RET_OK: +// - string-returning procs: (msg, len) is the raw string bytes (not +// NUL-terminated; use len). +// - struct-returning procs: msg is a pointer to the returned C struct — cast +// it to `const *` (len is sizeof). It is valid ONLY for the duration +// of the callback; copy out anything you need before returning. The library +// deep-frees it right after the callback (you free nothing). +// On RET_ERR, (msg, len) is the raw error text. A `_cbor` variant of each +// proc also exists for generic/cross-language callers that prefer CBOR. #ifndef NIM_FFI_GEN__H #define NIM_FFI_GEN__H diff --git a/ffi/ffi_thread_request.nim b/ffi/ffi_thread_request.nim index d609dba..3efc29f 100644 --- a/ffi/ffi_thread_request.nim +++ b/ffi/ffi_thread_request.nim @@ -43,6 +43,15 @@ type FFIThreadRequest* = object payloadFree*: PayloadFreeProc ## When non-nil, `data` is freed by calling this instead of `c_free` — used ## for the native C-POD payload, which owns its duplicated string fields. + respPod*: pointer + ## Native typed response: when non-nil, the handler produced a heap C-POD + ## struct (the `nimToPod` of a `{.ffi.}`-typed return) to hand to the + ## callback *instead of* the `res` bytes. The callback receives it as its + ## `msg` pointer (cast to `const *`) with `len = respPodLen`; it is + ## valid only for the callback's lifetime — `handleRes` deep-frees it via + ## `respPodFree` immediately after the callback returns. + respPodLen*: int + respPodFree*: PayloadFreeProc proc allocBaseRequest( callback: FFICallBack, userData: pointer, reqId: cstring @@ -58,6 +67,9 @@ proc allocBaseRequest( ret[].dataLen = 0 ret[].cborMode = true ret[].payloadFree = nil + ret[].respPod = nil + ret[].respPodLen = 0 + ret[].respPodFree = nil return ret proc copySharedPayload(req: ptr FFIThreadRequest, data: ptr byte, dataLen: int) = @@ -175,6 +187,21 @@ proc handleRes*(res: Result[seq[byte], string], request: ptr FFIThreadRequest) = ) return + # Native typed return: deliver the heap C-POD to the callback, then deep-free + # it (caller frees nothing). Takes precedence over the byte payload, which the + # handler leaves empty in this case. + if not request[].respPod.isNil(): + foreignThreadGc: + request[].callback( + RET_OK, + cast[ptr cchar](request[].respPod), + cast[csize_t](request[].respPodLen), + request[].userData, + ) + if request[].respPodFree != nil: + request[].respPodFree(request[].respPod) + return + foreignThreadGc: let bytes = res.get() if bytes.len > 0: diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index f30549d..5179bf7 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -996,18 +996,38 @@ macro ffi*(prc: untyped): untyped = let f = ident(extraParamNames[i]) ndBody.add(nativeArgUnpackStmt(ndCargs, f, extraParamTypes[i])) ndHelperCall.add(f) - ndBody.add quote do: - let `ndRet` = (await `ndHelperCall`).valueOr: - return err($error) - when typeof(`ndRet`) is string: - var rb = newSeq[byte](`ndRet`.len) - if `ndRet`.len > 0: - copyMem(addr rb[0], unsafeAddr `ndRet`[0], `ndRet`.len) - return ok(rb) - elif typeof(`ndRet`) is seq[byte]: - return ok(`ndRet`) - else: - return ok(cborEncode(`ndRet`)) + # A `{.ffi.}`-struct return travels back natively too: build its `Pod` + # mirror on the heap, hand it to the callback as a typed `const *`, and + # let handleRes deep-free it after the callback (caller frees nothing). Any + # other return (string -> raw bytes, seq[byte] -> raw, else -> CBOR) keeps + # the byte-payload path. + let retIsStruct = isFFIStructType(resultRetType) + let respPodFreeName = ident(camelName & "RespPodFree") + if retIsStruct: + let retPodType = ident($resultRetType & "Pod") + let ndPodPtr = genSym(nskLet, "respPod") + ndBody.add quote do: + let `ndRet` = (await `ndHelperCall`).valueOr: + return err($error) + let `ndPodPtr` = ffiCMalloc(`retPodType`) + `ndPodPtr`[] = nimToPod(`ndRet`) + `ndReq`[].respPod = cast[pointer](`ndPodPtr`) + `ndReq`[].respPodLen = sizeof(`retPodType`) + `ndReq`[].respPodFree = `respPodFreeName` + return ok(newSeq[byte](0)) + else: + ndBody.add quote do: + let `ndRet` = (await `ndHelperCall`).valueOr: + return err($error) + when typeof(`ndRet`) is string: + var rb = newSeq[byte](`ndRet`.len) + if `ndRet`.len > 0: + copyMem(addr rb[0], unsafeAddr `ndRet`[0], `ndRet`.len) + return ok(rb) + elif typeof(`ndRet`) is seq[byte]: + return ok(`ndRet`) + else: + return ok(cborEncode(`ndRet`)) let seqByteRet = nnkBracketExpr.newTree( ident("Future"), nnkBracketExpr.newTree( @@ -1031,6 +1051,30 @@ macro ffi*(prc: untyped): untyped = nativeHandlerProc, ) + # Per-proc destructor for the native typed response POD (only emitted when + # the return is a `{.ffi.}` struct). `freePod` recursively releases the + # duplicated strings / nested graphs; then the heap struct itself. + var respPodFreeProc: NimNode = newStmtList() + if retIsStruct: + let retPodType = ident($resultRetType & "Pod") + let fpP = genSym(nskParam, "p") + let fpPod = genSym(nskLet, "pod") + let respPodFreeBody = quote do: + let `fpPod` = cast[ptr `retPodType`](`fpP`) + freePod(`fpPod`[]) + ffiCFree(`fpP`) + respPodFreeProc = newProc( + name = respPodFreeName, + params = @[newEmptyNode(), newIdentDefs(fpP, ident("pointer"))], + body = respPodFreeBody, + pragmas = newTree( + nnkPragma, + ident("cdecl"), + newTree(nnkExprColonExpr, ident("raises"), newTree(nnkBracket)), + ident("gcsafe"), + ), + ) + # Native C export: build the C-POD (duplicating cstrings) and dispatch. let neCargs = genSym(nskLet, "cargs") let neReq = genSym(nskLet, "nreq") @@ -1121,8 +1165,8 @@ macro ffi*(prc: untyped): untyped = ) return newStmtList( - helperProc, registerReq, cargsTypeDef, cargsFreeProc, nativeRegister, - nativeExportProc, ffiProc, + helperProc, registerReq, cargsTypeDef, cargsFreeProc, respPodFreeProc, + nativeRegister, nativeExportProc, ffiProc, ) let stmts = newStmtList(flushPendingPods(), asyncPath())