Skip to content

_ctypes pt. 5 #5653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 3 commits into from
Nov 28, 2025
Merged

_ctypes pt. 5 #5653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f37ea52
Ctypes
arihant2math Apr 1, 2025
14cf4e3
Fix PyCSimple
youknowone Nov 28, 2025
8af105f
Make mergeable
youknowone Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion crates/vm/src/stdlib/ctypes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

pub(crate) mod array;
pub(crate) mod base;
pub(crate) mod field;
pub(crate) mod function;
pub(crate) mod library;
pub(crate) mod pointer;
pub(crate) mod structure;
pub(crate) mod thunk;
pub(crate) mod union;

use crate::builtins::PyModule;
Expand All @@ -17,14 +19,18 @@ pub fn extend_module_nodes(vm: &VirtualMachine, module: &Py<PyModule>) {
let ctx = &vm.ctx;
PyCSimpleType::make_class(ctx);
array::PyCArrayType::make_class(ctx);
field::PyCFieldType::make_class(ctx);
pointer::PyCPointerType::make_class(ctx);
extend_module!(vm, module, {
"_CData" => PyCData::make_class(ctx),
"_SimpleCData" => PyCSimple::make_class(ctx),
"Array" => array::PyCArray::make_class(ctx),
"CField" => field::PyCField::make_class(ctx),
"CFuncPtr" => function::PyCFuncPtr::make_class(ctx),
"_Pointer" => pointer::PyCPointer::make_class(ctx),
"_pointer_type_cache" => ctx.new_dict(),
"Structure" => structure::PyCStructure::make_class(ctx),
"CThunkObject" => thunk::PyCThunk::make_class(ctx),
"Union" => union::PyCUnion::make_class(ctx),
})
}
Expand Down Expand Up @@ -207,7 +213,9 @@ pub(crate) mod _ctypes {
// TODO: load_flags
let cache = library::libcache();
let mut cache_write = cache.write();
let (id, _) = cache_write.get_or_insert_lib(&name, vm).unwrap();
let (id, _) = cache_write
.get_or_insert_lib(&name, vm)
.map_err(|e| vm.new_os_error(e.to_string()))?;
Ok(id)
}

Expand Down
1 change: 1 addition & 0 deletions crates/vm/src/stdlib/ctypes/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ impl PyCArray {
}

impl PyCArray {
#[allow(unused)]
pub fn to_arg(&self, _vm: &VirtualMachine) -> PyResult<libffi::middle::Arg> {
let value = self.value.read();
let py_bytes = value.downcast_ref::<PyBytes>().unwrap();
Expand Down
27 changes: 14 additions & 13 deletions crates/vm/src/stdlib/ctypes/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,12 @@ impl Constructor for PyCSimple {
_ => vm.ctx.none(), // "z" | "Z" | "P"
}
};
Ok(PyCSimple {
PyCSimple {
_type_,
value: AtomicCell::new(value),
}
.to_pyobject(vm))
.into_ref_with_type(vm, cls)
.map(Into::into)
}
}

Expand Down Expand Up @@ -276,25 +277,25 @@ impl PyCSimple {
let value = unsafe { (*self.value.as_ptr()).clone() };
if let Ok(i) = value.try_int(vm) {
let i = i.as_bigint();
if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
return i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r));
return if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i8().as_raw_ptr()) {
return i.to_i8().map(|r: i8| libffi::middle::Arg::new(&r));
i.to_i8().map(|r: i8| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u16().as_raw_ptr()) {
return i.to_u16().map(|r: u16| libffi::middle::Arg::new(&r));
i.to_u16().map(|r: u16| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i16().as_raw_ptr()) {
return i.to_i16().map(|r: i16| libffi::middle::Arg::new(&r));
i.to_i16().map(|r: i16| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u32().as_raw_ptr()) {
return i.to_u32().map(|r: u32| libffi::middle::Arg::new(&r));
i.to_u32().map(|r: u32| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i32().as_raw_ptr()) {
return i.to_i32().map(|r: i32| libffi::middle::Arg::new(&r));
i.to_i32().map(|r: i32| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u64().as_raw_ptr()) {
return i.to_u64().map(|r: u64| libffi::middle::Arg::new(&r));
i.to_u64().map(|r: u64| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i64().as_raw_ptr()) {
return i.to_i64().map(|r: i64| libffi::middle::Arg::new(&r));
i.to_i64().map(|r: i64| libffi::middle::Arg::new(&r))
} else {
return None;
}
None
};
Comment on lines +280 to +298
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Use-after-free: Arg::new references stack-local values that are immediately dropped.

libffi::middle::Arg::new(&r) captures a reference to r, but r is a local variable created within the closure passed to map. Once the closure returns, r is dropped, leaving a dangling reference inside the Arg. This is undefined behavior when the Arg is later used in an FFI call.

You need to store the converted values in a longer-lived location. Consider storing the value in a field or using a boxed/heap-allocated approach:

     pub fn to_arg(
         &self,
         ty: libffi::middle::Type,
         vm: &VirtualMachine,
-    ) -> Option<libffi::middle::Arg> {
+    ) -> Option<(Box<dyn std::any::Any>, libffi::middle::Arg)> {
         let value = unsafe { (*self.value.as_ptr()).clone() };
         if let Ok(i) = value.try_int(vm) {
             let i = i.as_bigint();
-            return if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
-                i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r))
+            if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
+                return i.to_u8().map(|r| {
+                    let boxed: Box<u8> = Box::new(r);
+                    let arg = libffi::middle::Arg::new(boxed.as_ref());
+                    (boxed as Box<dyn std::any::Any>, arg)
+                });
+            }
             // ... similar pattern for other types

Alternatively, refactor to return an owned representation that the caller can hold until the FFI call completes.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
crates/vm/src/stdlib/ctypes/base.rs around lines 280-298: the current code calls
libffi::middle::Arg::new(&r) with r as a stack-local created inside the map
closure, producing a dangling reference after the closure returns; replace this
by producing owned, longer-lived storage for the converted value (for example
allocate the converted primitive on the heap with Box::new(...) or push it into
a Vec owned by the returned structure) and create the Arg from a pointer to that
owned storage, or refactor the function to return an owned wrapper (e.g., a
struct holding the boxed values plus the libffi::Arg pointers) so the converted
values outlive the Arg until the FFI call completes.

}
if let Ok(_f) = value.try_float(vm) {
todo!();
Expand Down
125 changes: 125 additions & 0 deletions crates/vm/src/stdlib/ctypes/field.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use crate::builtins::PyType;
use crate::builtins::PyTypeRef;
use crate::stdlib::ctypes::PyCData;
use crate::types::Constructor;
use crate::types::Representable;
use crate::{Py, PyResult, VirtualMachine};

#[pyclass(name = "PyCFieldType", base = PyType, module = "_ctypes")]
#[derive(PyPayload, Debug)]
pub struct PyCFieldType {
#[allow(dead_code)]
pub(super) inner: PyCField,
}

#[pyclass]
impl PyCFieldType {}

#[pyclass(
name = "CField",
base = PyCData,
metaclass = "PyCFieldType",
module = "_ctypes"
)]
#[derive(Debug, PyPayload)]
pub struct PyCField {
byte_offset: usize,
byte_size: usize,
#[allow(unused)]
index: usize,
proto: PyTypeRef,
anonymous: bool,
bitfield_size: bool,
bit_offset: u8,
name: String,
}

impl Representable for PyCField {
fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
let tp_name = zelf.proto.name().to_string();
if zelf.bitfield_size {
Ok(format!(
"<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}",
zelf.name,
tp_name,
byte_offset = zelf.byte_offset,
bitfield_size = zelf.bitfield_size,
bit_offset = zelf.bit_offset
))
} else {
Ok(format!(
"<{} type={tp_name}, ofs={}, size={}",
zelf.name, zelf.byte_offset, zelf.byte_size
))
}
}
}
Comment on lines +37 to +56
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix format strings and use idiomatic boolean check.

  1. The condition zelf.bitfield_size != false should use idiomatic zelf.bitfield_size.
  2. Both format strings are missing the closing > character.

Apply this diff:

 impl Representable for PyCField {
     fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
         let tp_name = zelf.proto.name().to_string();
-        if zelf.bitfield_size != false {
+        if zelf.bitfield_size {
             Ok(format!(
-                "<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}",
+                "<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}>",
                 zelf.name,
                 tp_name,
                 byte_offset = zelf.byte_offset,
                 bitfield_size = zelf.bitfield_size,
                 bit_offset = zelf.bit_offset
             ))
         } else {
             Ok(format!(
-                "<{} type={tp_name}, ofs={}, size={}",
+                "<{} type={tp_name}, ofs={}, size={}>",
                 zelf.name, zelf.byte_offset, zelf.byte_size
             ))
         }
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
impl Representable for PyCField {
fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
let tp_name = zelf.proto.name().to_string();
if zelf.bitfield_size != false {
Ok(format!(
"<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}",
zelf.name,
tp_name,
byte_offset = zelf.byte_offset,
bitfield_size = zelf.bitfield_size,
bit_offset = zelf.bit_offset
))
} else {
Ok(format!(
"<{} type={tp_name}, ofs={}, size={}",
zelf.name, zelf.byte_offset, zelf.byte_size
))
}
}
}
impl Representable for PyCField {
fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
let tp_name = zelf.proto.name().to_string();
if zelf.bitfield_size {
Ok(format!(
"<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}>",
zelf.name,
tp_name,
byte_offset = zelf.byte_offset,
bitfield_size = zelf.bitfield_size,
bit_offset = zelf.bit_offset
))
} else {
Ok(format!(
"<{} type={tp_name}, ofs={}, size={}>",
zelf.name, zelf.byte_offset, zelf.byte_size
))
}
}
}
🤖 Prompt for AI Agents 
In crates/vm/src/stdlib/ctypes/field.rs around lines 37–56, the boolean check
and two repr format strings are incorrect: change the condition from
`zelf.bitfield_size != false` to the idiomatic `zelf.bitfield_size` (or `if
zelf.bitfield_size {`), and add the missing trailing `>` character to both
format! string literals so each returned representation is properly closed; keep
the same placeholders and values when editing the format! calls.


#[derive(Debug, FromArgs)]
pub struct PyCFieldConstructorArgs {
// PyObject *name, PyObject *proto,
// Py_ssize_t byte_size, Py_ssize_t byte_offset,
// Py_ssize_t index, int _internal_use,
// PyObject *bit_size_obj, PyObject *bit_offset_obj
}

impl Constructor for PyCField {
type Args = PyCFieldConstructorArgs;

fn py_new(_cls: PyTypeRef, _args: Self::Args, vm: &VirtualMachine) -> PyResult {
Err(vm.new_type_error("Cannot instantiate a PyCField".to_string()))
}
}

#[pyclass(flags(BASETYPE, IMMUTABLETYPE), with(Constructor, Representable))]
impl PyCField {
#[pygetset]
fn size(&self) -> usize {
self.byte_size
}

#[pygetset]
fn bit_size(&self) -> bool {
self.bitfield_size
}

#[pygetset]
fn is_bitfield(&self) -> bool {
self.bitfield_size
}

#[pygetset]
fn is_anonymous(&self) -> bool {
self.anonymous
}

#[pygetset]
fn name(&self) -> String {
self.name.clone()
}

#[pygetset(name = "type")]
fn type_(&self) -> PyTypeRef {
self.proto.clone()
}

#[pygetset]
fn offset(&self) -> usize {
self.byte_offset
}

#[pygetset]
fn byte_offset(&self) -> usize {
self.byte_offset
}

#[pygetset]
fn byte_size(&self) -> usize {
self.byte_size
}

#[pygetset]
fn bit_offset(&self) -> u8 {
self.bit_offset
}
}
Loading
Loading