From 634fafdd805431ea4e56b7f076b0dcdb3a64efe0 Mon Sep 17 00:00:00 2001 From: Gabriel Cruz Date: Wed, 15 Jul 2026 15:36:53 -0300 Subject: [PATCH] fix: failing ci --- examples/echo/c_abi_bindings/echo.h | 24 ++++++++----- examples/echo/c_bindings/nim_ffi_cbor.h | 6 +++- examples/timer/c_bindings/nim_ffi_cbor.h | 6 +++- ffi/codegen/c.nim | 41 +++++++++++++++++----- ffi/codegen/templates/c/cbor_helpers.h.tpl | 6 +++- tests/unit/test_c_abi_codegen.nim | 8 +++-- 6 files changed, 69 insertions(+), 22 deletions(-) diff --git a/examples/echo/c_abi_bindings/echo.h b/examples/echo/c_abi_bindings/echo.h index 114ef5f..a7d7961 100644 --- a/examples/echo/c_abi_bindings/echo.h +++ b/examples/echo/c_abi_bindings/echo.h @@ -41,6 +41,20 @@ typedef void (*EchoCreateRawFn)(int err_code, const char* ctx_addr, const char* string return's UTF-8, or the 8-byte native-endian scalar image), not NUL-terminated and valid only for the duration of the call. */ typedef void (*EchoScalarRawFn)(int caller_ret, char* msg, size_t len, void* user_data); +#ifndef NIMFFI_ABI_DUP_CSTR_N +#define NIMFFI_ABI_DUP_CSTR_N +/* NUL-terminated copy of a length-delimited (not NUL-terminated) byte run; + NULL on allocation failure or a length that would overflow `n + 1`. */ +static inline char* nimffi_abi_dup_cstr_n(const char* s, size_t n) { + if (n == SIZE_MAX) return NULL; + char* p = (char*)malloc(n + 1); + if (p) { + if (n > 0) memcpy(p, s, n); + p[n] = '\0'; + } + return p; +} +#endif #ifdef __cplusplus extern "C" { #endif @@ -127,22 +141,16 @@ static void echo_version_scalar_reply(int caller_ret, char* msg, size_t len, voi free(box); if (!fn) return; if (caller_ret != NIMFFI_RET_OK) { - char* em = (char*)malloc(len + 1); - if (em) { - if (len > 0) memcpy(em, msg, len); - em[len] = '\0'; - } + char* em = nimffi_abi_dup_cstr_n(msg ? msg : "", msg ? len : 0); fn(caller_ret, "", em ? em : "FFI call failed", user_data); free(em); return; } - char* reply = (char*)malloc(len + 1); + char* reply = nimffi_abi_dup_cstr_n(msg ? msg : "", msg ? len : 0); if (!reply) { fn(NIMFFI_RET_ERR, "", "out of memory", user_data); return; } - if (len > 0) memcpy(reply, msg, len); - reply[len] = '\0'; fn(NIMFFI_RET_OK, reply, "", user_data); free(reply); } diff --git a/examples/echo/c_bindings/nim_ffi_cbor.h b/examples/echo/c_bindings/nim_ffi_cbor.h index 98d375c..63244aa 100644 --- a/examples/echo/c_bindings/nim_ffi_cbor.h +++ b/examples/echo/c_bindings/nim_ffi_cbor.h @@ -271,8 +271,12 @@ static inline char* nimffi_dup_cstr(const char* s) { } /* NUL-terminated copy of a length-delimited (not NUL-terminated) byte run, - * for turning the FFICallback's raw error `msg`/`len` into a C string. */ + * for turning the FFICallback's raw error `msg`/`len` into a C string. Returns + * NULL on allocation failure or a length that would overflow `n + 1`. */ static inline char* nimffi_dup_cstr_n(const char* s, size_t n) { + if (n == SIZE_MAX) { + return NULL; + } char* p = (char*)malloc(n + 1); if (p) { if (n > 0) { diff --git a/examples/timer/c_bindings/nim_ffi_cbor.h b/examples/timer/c_bindings/nim_ffi_cbor.h index 98d375c..63244aa 100644 --- a/examples/timer/c_bindings/nim_ffi_cbor.h +++ b/examples/timer/c_bindings/nim_ffi_cbor.h @@ -271,8 +271,12 @@ static inline char* nimffi_dup_cstr(const char* s) { } /* NUL-terminated copy of a length-delimited (not NUL-terminated) byte run, - * for turning the FFICallback's raw error `msg`/`len` into a C string. */ + * for turning the FFICallback's raw error `msg`/`len` into a C string. Returns + * NULL on allocation failure or a length that would overflow `n + 1`. */ static inline char* nimffi_dup_cstr_n(const char* s, size_t n) { + if (n == SIZE_MAX) { + return NULL; + } char* p = (char*)malloc(n + 1); if (p) { if (n > 0) { diff --git a/ffi/codegen/c.nim b/ffi/codegen/c.nim index d76bd2b..1e3ca40 100644 --- a/ffi/codegen/c.nim +++ b/ffi/codegen/c.nim @@ -1018,6 +1018,31 @@ func abiScalarRawFnName(libType: string): string = ## per-method trampoline converts into the typed reply. return libType & "ScalarRawFn" +const abiScalarDupCStr = "nimffi_abi_dup_cstr_n" + +func abiScalarDupHelper(): seq[string] = + ## CBOR-free twin of `nimffi_dup_cstr_n` for the scalar trampolines: a + ## NUL-terminated copy of a length-delimited (not NUL-terminated) byte run, + ## returning NULL on allocation failure or a length that would overflow the + ## `n + 1` size passed to `malloc`. + # Guarded so co-including two `abi = c` headers in one TU doesn't redefine it. + return @[ + "#ifndef NIMFFI_ABI_DUP_CSTR_N", + "#define NIMFFI_ABI_DUP_CSTR_N", + "/* NUL-terminated copy of a length-delimited (not NUL-terminated) byte run;", + " NULL on allocation failure or a length that would overflow `n + 1`. */", + "static inline char* " & abiScalarDupCStr & "(const char* s, size_t n) {", + " if (n == SIZE_MAX) return NULL;", + " char* p = (char*)malloc(n + 1);", + " if (p) {", + " if (n > 0) memcpy(p, s, n);", + " p[n] = '\\0';", + " }", + " return p;", + "}", + "#endif", + ] + func abiScalarArgParams(m: FFIProcMeta): seq[string] = ## C parameters for a scalar method's args — passed inline by value, in both ## the raw export and the high-level wrapper (no Req struct). @@ -1054,6 +1079,8 @@ proc emitAbiExternDecls( "typedef void (*" & abiScalarRawFnName(libType) & ")(int caller_ret, char* msg, size_t len, void* user_data);" ) + for l in abiScalarDupHelper(): + lines.add(l) lines.add("#ifdef __cplusplus") lines.add("extern \"C\" {") lines.add("#endif") @@ -1221,10 +1248,10 @@ func abiScalarOkLines(m: FFIProcMeta, fnType: string): seq[string] = let rt = m.returnTypeName.strip() if rt == "string" or rt == "cstring": return @[ - " char* reply = (char*)malloc(len + 1);", " if (!reply) {", + " char* reply = " & abiScalarDupCStr & "(msg ? msg : \"\", msg ? len : 0);", + " if (!reply) {", " fn(NIMFFI_RET_ERR, \"\", \"out of memory\", user_data);", - " return;", " }", " if (len > 0) memcpy(reply, msg, len);", - " reply[len] = '\\0';", " fn(NIMFFI_RET_OK, reply, \"\", user_data);", + " return;", " }", " fn(NIMFFI_RET_OK, reply, \"\", user_data);", " free(reply);", ] var lines = @[ @@ -1294,11 +1321,9 @@ proc emitAbiScalarMethod( lines.add(" free(box);") lines.add(" if (!fn) return;") lines.add(" if (caller_ret != NIMFFI_RET_OK) {") - lines.add(" char* em = (char*)malloc(len + 1);") - lines.add(" if (em) {") - lines.add(" if (len > 0) memcpy(em, msg, len);") - lines.add(" em[len] = '\\0';") - lines.add(" }") + lines.add( + " char* em = " & abiScalarDupCStr & "(msg ? msg : \"\", msg ? len : 0);" + ) lines.add( " fn(caller_ret, " & errReply & ", em ? em : \"FFI call failed\", user_data);" ) diff --git a/ffi/codegen/templates/c/cbor_helpers.h.tpl b/ffi/codegen/templates/c/cbor_helpers.h.tpl index 3a5e29e..8afdba7 100644 --- a/ffi/codegen/templates/c/cbor_helpers.h.tpl +++ b/ffi/codegen/templates/c/cbor_helpers.h.tpl @@ -271,8 +271,12 @@ static inline char* nimffi_dup_cstr(const char* s) { } /* NUL-terminated copy of a length-delimited (not NUL-terminated) byte run, - * for turning the FFICallback's raw error `msg`/`len` into a C string. */ + * for turning the FFICallback's raw error `msg`/`len` into a C string. Returns + * NULL on allocation failure or a length that would overflow `n + 1`. */ static inline char* nimffi_dup_cstr_n(const char* s, size_t n) { + if (n == SIZE_MAX) { + return NULL; + } char* p = (char*)malloc(n + 1); if (p) { if (n > 0) { diff --git a/tests/unit/test_c_abi_codegen.nim b/tests/unit/test_c_abi_codegen.nim index ef951dc..c8a5896 100644 --- a/tests/unit/test_c_abi_codegen.nim +++ b/tests/unit/test_c_abi_codegen.nim @@ -160,13 +160,15 @@ suite "generateCAbiLibHeader": header check "return timer_add(ctx->ptr, timer_add_scalar_reply, box, a, b);" in header - test "scalar returns unpack the 8-byte image; strings copy and NUL-terminate": + test "scalar returns unpack the 8-byte image; strings copy through the dup helper": # int return: the slot is the sign-extended int64 image. check "memcpy(&reply, &slot, sizeof(reply));" in header # float32 return: packed as a widened double, narrowed back here. check "float reply = (float)wide;" in header - # string return: raw UTF-8 bytes, not NUL-terminated on the wire. - check "reply[len] = '\\0';" in header + # string return: copied via the overflow-guarded, NUL-terminating dup helper. + check "char* reply = nimffi_abi_dup_cstr_n(msg ? msg : \"\", msg ? len : 0);" in + header + check "if (n == SIZE_MAX) return NULL;" in header test "events are rejected (CBOR-only for now)": expect ValueError: