From 5767997e0e86e5b20f113b67cc27f2325e05df89 Mon Sep 17 00:00:00 2001 From: osmaczko <33099791+osmaczko@users.noreply.github.com> Date: Tue, 24 Feb 2026 12:43:10 +0100 Subject: [PATCH] fix(nim-bindings): bridge Nim/C ABI mismatch via C shim layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nim's code generator transforms function signatures involving large structs in two ways that conflict with the standard C ABI: - Return values of large structs (> register size): Nim emits a void function with an explicit out-pointer appended as the *last* argument. The standard x86-64 SysV ABI passes the hidden return pointer in RDI (before the real arguments); ARM64 aapcs64 uses X8. Calling Rust directly from Nim therefore puts the pointer in the wrong register / stack slot on both architectures, causing crashes. - Large struct parameters (> ~24 bytes): Nim passes a pointer rather than copying bytes on the stack / into registers as the C ABI expects. This commit introduces a thin C shim (nim_shims.c) that acts as a translation layer: - Each nim_* wrapper is declared with a Nim-compatible signature, so Nim calls it correctly by its own rules. - Inside the wrapper the C compiler calls the real Rust-exported function using the standard C ABI, inserting the correct hidden- pointer placement and stack-copy behaviour for the current platform. As a result: - The Rust API stays standard C ABI (return T by value; destroy takes *mut T, which is pointer-sized and matches Nim's large-param transform). - Other language bindings (C, Swift, Go, …) call Rust directly without any shim — the standard ABI is preserved for them. - The fix is correct on both x86-64 and ARM64 without any architecture-specific code in Nim or Rust. Changes: - nim-bindings/src/nim_shims.c: C bridge with nim_* wrappers for all create/handle/installation_name and destroy functions - nim-bindings/src/bindings.nim: {.compile: "nim_shims.c"}, proc signatures use natural return-by-value form, importc names point to the nim_* shims - nim-bindings/src/libchat.nim: call sites use natural let binding form; destroy calls pass addr res (ptr T) - conversations/src/api.rs: destroy functions take *mut T so Nim's large-param-to-pointer transform is satisfied without a stack copy --- conversations/src/api.rs | 24 ++++++---- nim-bindings/src/bindings.nim | 32 ++++--------- nim-bindings/src/libchat.nim | 44 +++++++++-------- nim-bindings/src/nim_shims.c | 89 +++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 53 deletions(-) create mode 100644 nim-bindings/src/nim_shims.c diff --git a/conversations/src/api.rs b/conversations/src/api.rs index 62d635c..0842910 100644 --- a/conversations/src/api.rs +++ b/conversations/src/api.rs @@ -196,8 +196,10 @@ pub struct CreateIntroResult { /// Free the result from create_intro_bundle #[ffi_export] -pub fn destroy_intro_result(result: CreateIntroResult) { - drop(result); +pub fn destroy_intro_result(result: *mut CreateIntroResult) { + if !result.is_null() { + unsafe { std::ptr::drop_in_place(result) } + } } /// Payload structure for FFI @@ -220,8 +222,10 @@ pub struct SendContentResult { /// Free the result from send_content #[ffi_export] -pub fn destroy_send_content_result(result: SendContentResult) { - drop(result); +pub fn destroy_send_content_result(result: *mut SendContentResult) { + if !result.is_null() { + unsafe { std::ptr::drop_in_place(result) } + } } /// Result structure for handle_payload @@ -238,8 +242,10 @@ pub struct HandlePayloadResult { /// Free the result from handle_payload #[ffi_export] -pub fn destroy_handle_payload_result(result: HandlePayloadResult) { - drop(result); +pub fn destroy_handle_payload_result(result: *mut HandlePayloadResult) { + if !result.is_null() { + unsafe { std::ptr::drop_in_place(result) } + } } impl From for HandlePayloadResult { @@ -292,6 +298,8 @@ pub struct NewConvoResult { /// Free the result from create_new_private_convo #[ffi_export] -pub fn destroy_convo_result(result: NewConvoResult) { - drop(result); +pub fn destroy_convo_result(result: *mut NewConvoResult) { + if !result.is_null() { + unsafe { std::ptr::drop_in_place(result) } + } } diff --git a/nim-bindings/src/bindings.nim b/nim-bindings/src/bindings.nim index 569e81a..b1e533e 100644 --- a/nim-bindings/src/bindings.nim +++ b/nim-bindings/src/bindings.nim @@ -1,4 +1,5 @@ # Nim FFI bindings for libchat conversations library +{.compile: "nim_shims.c".} # Error codes (must match Rust ErrorCode enum) const @@ -77,7 +78,7 @@ proc create_context*(name: ReprCString): ContextHandle {.importc.} ## Returns the friendly name of the context's identity ## The result must be freed by the caller (repr_c::String ownership transfers) -proc installation_name*(ctx: ContextHandle): ReprCString {.importc.} +proc installation_name*(ctx: ContextHandle): ReprCString {.importc: "nim_installation_name".} ## Destroys a context and frees its memory ## - handle must be a valid pointer from create_context() @@ -87,49 +88,36 @@ proc destroy_context*(ctx: ContextHandle) {.importc.} ## Creates an intro bundle for sharing with other users ## Returns: CreateIntroResult struct - check error_code field (0 = success, negative = error) ## The result must be freed with destroy_intro_result() -proc create_intro_bundle*( - ctx: ContextHandle, -): CreateIntroResult {.importc.} +proc create_intro_bundle*(ctx: ContextHandle): CreateIntroResult {.importc: "nim_create_intro_bundle".} ## Creates a new private conversation ## Returns: NewConvoResult struct - check error_code field (0 = success, negative = error) ## The result must be freed with destroy_convo_result() -proc create_new_private_convo*( - ctx: ContextHandle, - bundle: SliceUint8, - content: SliceUint8, -): NewConvoResult {.importc.} +proc create_new_private_convo*(ctx: ContextHandle, bundle: SliceUint8, content: SliceUint8): NewConvoResult {.importc: "nim_create_new_private_convo".} ## Sends content to an existing conversation ## Returns: SendContentResult struct - check error_code field (0 = success, negative = error) ## The result must be freed with destroy_send_content_result() -proc send_content*( - ctx: ContextHandle, - convo_id: ReprCString, - content: SliceUint8, -): SendContentResult {.importc.} +proc send_content*(ctx: ContextHandle, convo_id: ReprCString, content: SliceUint8): SendContentResult {.importc: "nim_send_content".} ## Handles an incoming payload ## Returns: HandlePayloadResult struct - check error_code field (0 = success, negative = error) ## This call does not always generate content. If content is zero bytes long then there ## is no data, and the convo_id should be ignored. ## The result must be freed with destroy_handle_payload_result() -proc handle_payload*( - ctx: ContextHandle, - payload: SliceUint8, -): HandlePayloadResult {.importc.} +proc handle_payload*(ctx: ContextHandle, payload: SliceUint8): HandlePayloadResult {.importc: "nim_handle_payload".} ## Free the result from create_intro_bundle -proc destroy_intro_result*(result: CreateIntroResult) {.importc.} +proc destroy_intro_result*(result: ptr CreateIntroResult) {.importc: "nim_destroy_intro_result".} ## Free the result from create_new_private_convo -proc destroy_convo_result*(result: NewConvoResult) {.importc.} +proc destroy_convo_result*(result: ptr NewConvoResult) {.importc: "nim_destroy_convo_result".} ## Free the result from send_content -proc destroy_send_content_result*(result: SendContentResult) {.importc.} +proc destroy_send_content_result*(result: ptr SendContentResult) {.importc: "nim_destroy_send_content_result".} ## Free the result from handle_payload -proc destroy_handle_payload_result*(result: HandlePayloadResult) {.importc.} +proc destroy_handle_payload_result*(result: ptr HandlePayloadResult) {.importc: "nim_destroy_handle_payload_result".} # ============================================================================ # Helper functions diff --git a/nim-bindings/src/libchat.nim b/nim-bindings/src/libchat.nim index 9b2cb0e..a499b77 100644 --- a/nim-bindings/src/libchat.nim +++ b/nim-bindings/src/libchat.nim @@ -43,14 +43,15 @@ proc createIntroductionBundle*(ctx: LibChat): Result[seq[byte], string] = if ctx.handle == nil: return err("Context handle is nil") - let res = create_intro_bundle(ctx.handle) + var res = create_intro_bundle(ctx.handle) if res.error_code != ErrNone: - result = err("Failed to create private convo: " & $res.error_code) - destroy_intro_result(res) - return + destroy_intro_result(addr res) + return err("Failed to create intro bundle: " & $res.error_code) - return ok(res.intro_bytes.toSeq()) + let intro = res.intro_bytes.toSeq() + destroy_intro_result(addr res) + return ok(intro) ## Create a Private Convo proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte]): Result[(string, seq[PayloadResult]), string] = @@ -62,16 +63,15 @@ proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte]) if content.len == 0: return err("content is zero length") - let res = bindings.create_new_private_convo( + var res = bindings.create_new_private_convo( ctx.handle, bundle.toSlice(), content.toSlice() ) if res.error_code != 0: - result = err("Failed to create private convo: " & $res.error_code) - destroy_convo_result(res) - return + destroy_convo_result(addr res) + return err("Failed to create private convo: " & $res.error_code) # Convert payloads to Nim types var payloads = newSeq[PayloadResult](res.payloads.len) @@ -85,7 +85,7 @@ proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte]) let convoId = $res.convo_id # Free the result - destroy_convo_result(res) + destroy_convo_result(addr res) return ok((convoId, payloads)) @@ -97,24 +97,22 @@ proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq if content.len == 0: return err("content is zero length") - let res = bindings.send_content( + var res = bindings.send_content( ctx.handle, convoId.toReprCString, content.toSlice() ) if res.error_code != 0: - result = err("Failed to send content: " & $res.error_code) - destroy_send_content_result(res) - return - + destroy_send_content_result(addr res) + return err("Failed to send content: " & $res.error_code) let payloads = res.payloads.toSeq().mapIt(PayloadResult( address: $it.address, data: it.data.toSeq() )) - destroy_send_content_result(res) + destroy_send_content_result(addr res) return ok(payloads) type @@ -131,24 +129,24 @@ proc handlePayload*(ctx: LibChat, payload: seq[byte]): Result[Option[ContentResu if payload.len == 0: return err("payload is zero length") - var conversationIdBuf = newSeq[byte](ctx.buffer_size) - var contentBuf = newSeq[byte](ctx.buffer_size) - var conversationIdLen: uint32 = 0 - - let res = bindings.handle_payload( + var res = bindings.handle_payload( ctx.handle, payload.toSlice(), ) if res.error_code != ErrNone: + destroy_handle_payload_result(addr res) return err("Failed to handle payload: " & $res.error_code) let content = res.content.toSeq() if content.len == 0: + destroy_handle_payload_result(addr res) return ok(none(ContentResult)) - return ok(some(ContentResult( + let r = some(ContentResult( conversationId: $res.convo_id, data: content, isNewConvo: res.is_new_convo - ))) + )) + destroy_handle_payload_result(addr res) + return ok(r) diff --git a/nim-bindings/src/nim_shims.c b/nim-bindings/src/nim_shims.c new file mode 100644 index 0000000..628fec7 --- /dev/null +++ b/nim-bindings/src/nim_shims.c @@ -0,0 +1,89 @@ +/* nim_shims.c — bridges Nim's calling convention to the standard C ABI. + * + * Nim transforms functions returning large structs (> register size) into + * void functions with an explicit out-pointer appended as the last argument. + * It also transforms large struct parameters (> ~24 bytes) into pointers. + * These transformations do not match the x86-64 SysV hidden-return-pointer + * convention (RDI) or ARM64 aapcs64 (X8), causing crashes. + * + * Each nim_* wrapper has a Nim-compatible signature. The C compiler handles + * the correct hidden-pointer and stack-copy conventions when calling the + * underlying Rust-exported functions. + */ + +#include +#include +#include + +typedef void* ContextHandle; + +/* Matches c_slice::Ref<'_, u8> — 16 bytes, passed in registers */ +typedef struct { const uint8_t* ptr; size_t len; } SliceRefU8; + +/* Matches repr_c::Vec — 24 bytes */ +typedef struct { uint8_t* ptr; size_t len; size_t cap; } VecU8; + +/* Matches repr_c::String — 24 bytes */ +typedef struct { char* ptr; size_t len; size_t cap; } ReprCString; + +/* Matches FFI Payload — 48 bytes */ +typedef struct { ReprCString address; VecU8 data; } Payload; + +/* Matches repr_c::Vec — 24 bytes */ +typedef struct { Payload* ptr; size_t len; size_t cap; } VecPayload; + +/* 32 bytes — Nim transforms: parameter → ptr, return → out-ptr at end */ +typedef struct { int32_t error_code; VecU8 intro_bytes; } CreateIntroResult; + +/* 56 bytes */ +typedef struct { int32_t error_code; ReprCString convo_id; VecPayload payloads; } NewConvoResult; + +/* 32 bytes */ +typedef struct { int32_t error_code; VecPayload payloads; } SendContentResult; + +/* 64 bytes */ +typedef struct { + int32_t error_code; ReprCString convo_id; VecU8 content; bool is_new_convo; +} HandlePayloadResult; + +/* Forward declarations — Rust-exported functions, standard C ABI */ +extern CreateIntroResult create_intro_bundle(ContextHandle ctx); +extern NewConvoResult create_new_private_convo(ContextHandle ctx, SliceRefU8 bundle, SliceRefU8 content); +extern SendContentResult send_content(ContextHandle ctx, ReprCString convo_id, SliceRefU8 content); +extern HandlePayloadResult handle_payload(ContextHandle ctx, SliceRefU8 payload); +extern ReprCString installation_name(ContextHandle ctx); +extern void destroy_intro_result(CreateIntroResult* result); /* *mut T */ +extern void destroy_convo_result(NewConvoResult* result); +extern void destroy_send_content_result(SendContentResult* result); +extern void destroy_handle_payload_result(HandlePayloadResult* result); + +/* Return-value wrappers: C compiler inserts correct hidden-pointer per platform */ +void nim_create_intro_bundle(ContextHandle ctx, CreateIntroResult* out) { + *out = create_intro_bundle(ctx); +} +void nim_create_new_private_convo(ContextHandle ctx, SliceRefU8 bundle, SliceRefU8 content, NewConvoResult* out) { + *out = create_new_private_convo(ctx, bundle, content); +} +void nim_send_content(ContextHandle ctx, ReprCString convo_id, SliceRefU8 content, SendContentResult* out) { + *out = send_content(ctx, convo_id, content); +} +void nim_handle_payload(ContextHandle ctx, SliceRefU8 payload, HandlePayloadResult* out) { + *out = handle_payload(ctx, payload); +} +void nim_installation_name(ContextHandle ctx, ReprCString* out) { + *out = installation_name(ctx); +} + +/* Destroy wrappers: Nim passes pointer (for > 24-byte params); forward to Rust *mut T */ +void nim_destroy_intro_result(CreateIntroResult* result) { + destroy_intro_result(result); +} +void nim_destroy_convo_result(NewConvoResult* result) { + destroy_convo_result(result); +} +void nim_destroy_send_content_result(SendContentResult* result) { + destroy_send_content_result(result); +} +void nim_destroy_handle_payload_result(HandlePayloadResult* result) { + destroy_handle_payload_result(result); +}