mirror of
https://github.com/logos-messaging/libchat.git
synced 2026-05-02 00:03:11 +00:00
fix(nim-bindings): fix ABI mismatch in destroy_* FFI functions and add defer-based cleanup
Nim's C backend silently transforms large struct parameters (>16 bytes) into pointer parameters when calling importc functions. The destroy_* functions were declared taking T by value in Rust, but Nim always passed &T — causing Rust to read garbage from the stack on x86-64 (SIGILL on CI) while accidentally working on ARM64 macOS due to that ABI coincidentally also using pointers for large structs. Fix by changing all destroy_* functions to take &mut T and using drop_in_place, which is the correct idiom for dropping a value through a pointer. On the Nim side, replace scattered manual destroy calls with defer, which guarantees cleanup on all exit paths and prevents use-after-destroy bugs.
This commit is contained in:
parent
f150619b17
commit
1f8c38d2ce
@ -214,9 +214,13 @@ pub struct CreateIntroResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Free the result from create_intro_bundle
|
/// Free the result from create_intro_bundle
|
||||||
|
///
|
||||||
|
/// # ABI note
|
||||||
|
/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
|
||||||
|
/// accepting the struct by value would be an ABI mismatch on the caller side.
|
||||||
#[ffi_export]
|
#[ffi_export]
|
||||||
pub fn destroy_intro_result(result: CreateIntroResult) {
|
pub fn destroy_intro_result(result: &mut CreateIntroResult) {
|
||||||
drop(result);
|
unsafe { std::ptr::drop_in_place(result) }
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Payload structure for FFI
|
/// Payload structure for FFI
|
||||||
@ -238,9 +242,13 @@ pub struct SendContentResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Free the result from send_content
|
/// Free the result from send_content
|
||||||
|
///
|
||||||
|
/// # ABI note
|
||||||
|
/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
|
||||||
|
/// accepting the struct by value would be an ABI mismatch on the caller side.
|
||||||
#[ffi_export]
|
#[ffi_export]
|
||||||
pub fn destroy_send_content_result(result: SendContentResult) {
|
pub fn destroy_send_content_result(result: &mut SendContentResult) {
|
||||||
drop(result);
|
unsafe { std::ptr::drop_in_place(result) }
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Result structure for handle_payload
|
/// Result structure for handle_payload
|
||||||
@ -256,9 +264,13 @@ pub struct HandlePayloadResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Free the result from handle_payload
|
/// Free the result from handle_payload
|
||||||
|
///
|
||||||
|
/// # ABI note
|
||||||
|
/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
|
||||||
|
/// accepting the struct by value would be an ABI mismatch on the caller side.
|
||||||
#[ffi_export]
|
#[ffi_export]
|
||||||
pub fn destroy_handle_payload_result(result: HandlePayloadResult) {
|
pub fn destroy_handle_payload_result(result: &mut HandlePayloadResult) {
|
||||||
drop(result);
|
unsafe { std::ptr::drop_in_place(result) }
|
||||||
}
|
}
|
||||||
|
|
||||||
impl From<ContentData> for HandlePayloadResult {
|
impl From<ContentData> for HandlePayloadResult {
|
||||||
@ -310,7 +322,11 @@ pub struct NewConvoResult {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Free the result from create_new_private_convo
|
/// Free the result from create_new_private_convo
|
||||||
|
///
|
||||||
|
/// # ABI note
|
||||||
|
/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
|
||||||
|
/// accepting the struct by value would be an ABI mismatch on the caller side.
|
||||||
#[ffi_export]
|
#[ffi_export]
|
||||||
pub fn destroy_convo_result(result: NewConvoResult) {
|
pub fn destroy_convo_result(result: &mut NewConvoResult) {
|
||||||
drop(result);
|
unsafe { std::ptr::drop_in_place(result) }
|
||||||
}
|
}
|
||||||
|
|||||||
@ -44,11 +44,10 @@ proc createIntroductionBundle*(ctx: LibChat): Result[seq[byte], string] =
|
|||||||
return err("Context handle is nil")
|
return err("Context handle is nil")
|
||||||
|
|
||||||
let res = create_intro_bundle(ctx.handle)
|
let res = create_intro_bundle(ctx.handle)
|
||||||
|
defer: destroy_intro_result(res)
|
||||||
|
|
||||||
if res.error_code != ErrNone:
|
if res.error_code != ErrNone:
|
||||||
result = err("Failed to create private convo: " & $res.error_code)
|
return err("Failed to create intro bundle: " & $res.error_code)
|
||||||
destroy_intro_result(res)
|
|
||||||
return
|
|
||||||
|
|
||||||
return ok(res.intro_bytes.toSeq())
|
return ok(res.intro_bytes.toSeq())
|
||||||
|
|
||||||
@ -62,32 +61,18 @@ proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte])
|
|||||||
if content.len == 0:
|
if content.len == 0:
|
||||||
return err("content is zero length")
|
return err("content is zero length")
|
||||||
|
|
||||||
let res = bindings.create_new_private_convo(
|
let res = bindings.create_new_private_convo(ctx.handle, bundle.toSlice(), content.toSlice())
|
||||||
ctx.handle,
|
defer: destroy_convo_result(res)
|
||||||
bundle.toSlice(),
|
|
||||||
content.toSlice()
|
|
||||||
)
|
|
||||||
|
|
||||||
if res.error_code != 0:
|
if res.error_code != 0:
|
||||||
result = err("Failed to create private convo: " & $res.error_code)
|
return err("Failed to create private convo: " & $res.error_code)
|
||||||
destroy_convo_result(res)
|
|
||||||
return
|
|
||||||
|
|
||||||
# Convert payloads to Nim types
|
|
||||||
var payloads = newSeq[PayloadResult](res.payloads.len)
|
var payloads = newSeq[PayloadResult](res.payloads.len)
|
||||||
for i in 0 ..< res.payloads.len:
|
for i in 0 ..< res.payloads.len:
|
||||||
let p = res.payloads[int(i)]
|
let p = res.payloads[int(i)]
|
||||||
payloads[int(i)] = PayloadResult(
|
payloads[int(i)] = PayloadResult(address: $p.address, data: p.data.toSeq())
|
||||||
address: $p.address,
|
|
||||||
data: p.data.toSeq()
|
|
||||||
)
|
|
||||||
|
|
||||||
let convoId = $res.convo_id
|
return ok(($res.convo_id, payloads))
|
||||||
|
|
||||||
# Free the result
|
|
||||||
destroy_convo_result(res)
|
|
||||||
|
|
||||||
return ok((convoId, payloads))
|
|
||||||
|
|
||||||
## Send content to an existing conversation
|
## Send content to an existing conversation
|
||||||
proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq[PayloadResult], string] =
|
proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq[PayloadResult], string] =
|
||||||
@ -97,24 +82,13 @@ proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq
|
|||||||
if content.len == 0:
|
if content.len == 0:
|
||||||
return err("content is zero length")
|
return err("content is zero length")
|
||||||
|
|
||||||
let res = bindings.send_content(
|
let res = bindings.send_content(ctx.handle, convoId.toReprCString, content.toSlice())
|
||||||
ctx.handle,
|
defer: destroy_send_content_result(res)
|
||||||
convoId.toReprCString,
|
|
||||||
content.toSlice()
|
|
||||||
)
|
|
||||||
|
|
||||||
if res.error_code != 0:
|
if res.error_code != 0:
|
||||||
result = err("Failed to send content: " & $res.error_code)
|
return err("Failed to send content: " & $res.error_code)
|
||||||
destroy_send_content_result(res)
|
|
||||||
return
|
|
||||||
|
|
||||||
|
let payloads = res.payloads.toSeq().mapIt(PayloadResult(address: $it.address, data: it.data.toSeq()))
|
||||||
let payloads = res.payloads.toSeq().mapIt(PayloadResult(
|
|
||||||
address: $it.address,
|
|
||||||
data: it.data.toSeq()
|
|
||||||
))
|
|
||||||
|
|
||||||
destroy_send_content_result(res)
|
|
||||||
return ok(payloads)
|
return ok(payloads)
|
||||||
|
|
||||||
type
|
type
|
||||||
@ -131,14 +105,8 @@ proc handlePayload*(ctx: LibChat, payload: seq[byte]): Result[Option[ContentResu
|
|||||||
if payload.len == 0:
|
if payload.len == 0:
|
||||||
return err("payload is zero length")
|
return err("payload is zero length")
|
||||||
|
|
||||||
var conversationIdBuf = newSeq[byte](ctx.buffer_size)
|
let res = bindings.handle_payload(ctx.handle, payload.toSlice())
|
||||||
var contentBuf = newSeq[byte](ctx.buffer_size)
|
defer: destroy_handle_payload_result(res)
|
||||||
var conversationIdLen: uint32 = 0
|
|
||||||
|
|
||||||
let res = bindings.handle_payload(
|
|
||||||
ctx.handle,
|
|
||||||
payload.toSlice(),
|
|
||||||
)
|
|
||||||
|
|
||||||
if res.error_code != ErrNone:
|
if res.error_code != ErrNone:
|
||||||
return err("Failed to handle payload: " & $res.error_code)
|
return err("Failed to handle payload: " & $res.error_code)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user