Skip to content

ctypes: unify the foreign-call path on a single host_env call() API - #8235

Merged
youknowone merged 8 commits into
RustPython:mainfrom
youknowone:hostenv-ctypes-call
Jul 8, 2026
Merged

ctypes: unify the foreign-call path on a single host_env call() API#8235
youknowone merged 8 commits into
RustPython:mainfrom
youknowone:hostenv-ctypes-call

Conversation

@youknowone

@youknowone youknowone commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

Introduce a single libffi-hiding foreign-call entry point in host_env
(ctypes::call) and route the VM _ctypes module through it, replacing the
per-call libffi Type/Arg marshalling. On top of that, pass and return
Structure/Union by value, and delete the now-unused pre-call scaffolding.

Commits

  1. host_env: add unified high-level call foreign-call API — a
    call(addr, &[CallArg], CallRet, CallOptions) -> Result<CallValue, CallError>
    entry point that takes ctypes type codes and recursive CTypeLayouts plus
    raw buffers instead of libffi Type/Arg. Built on the existing
    Cif/ffi_* primitives; aggregate returns go through Cif::call_return_into.
    Adds 16 ABI unit tests over local extern "C" functions.
  2. _ctypes: route the VM call path through host_env call() — a
    behavior-preserving migration of ctypes_callproc and the argument/return
    marshalling onto call; the errno / last-error swap moves into CallOptions.
  3. _ctypes: pass and return structs and unions by value — struct/union
    arguments lower to CallArg::Aggregate and struct/union returns to
    CallRet::Aggregate. StgInfo carries per-field CTypeLayouts built
    incrementally from the base class, so struct inheritance is reflected. Also
    fixes a latent self-deadlock in the result path (a restype StgInfo read
    guard was held across result-instance construction, which write-locks the
    same StgInfo to finalize it — unreachable until a struct was actually
    returned by value).
  4. host_env: remove the pre-call foreign-call scaffolding — delete the
    old callproc / callproc_simple paths and the libffi-type builders
    (ffi_type_for_layout, CTypeParamKind, ffi_type_from_tag, …) that no
    longer have a consumer.

Testing

  • python -m test test_ctypes: run=322 skipped=58, unchanged across all four
    commits.
  • New extra_tests/snippets/stdlib_ctypes_calls.py (libc calls over the live
    FFI path) and stdlib_ctypes_byvalue.py (div/imaxdiv struct returns,
    inet_ntoa struct and union arguments); output matches CPython.
  • host_env ABI unit tests, workspace test suite, clippy, and pre-commit all
    pass.

Known limitation carried over from the previous libffi field-type path:
_pack_ / _swappedbytes_ layouts stay approximate, since CTypeLayout
carries no explicit field offsets.

Opening as a draft for early visibility / CI.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • ctypes now supports additional native call patterns, including passing and returning structs/unions by value.
    • Improved foreign-call handling for simple scalar, pointer, and array types.
    • Added mechanisms to preserve referenced buffer memory across calls.
  • Bug Fixes

    • Fixed foreign-call result decoding, including aggregate return values.
    • Improved error handling for null function pointers, unsupported/unknown type codes, and short aggregate buffers.
    • Enhanced errno/last-error behavior during calls.
  • Tests

    • Added new ctypes integration and regression coverage for by-value and call/errno scenarios.

Add a libffi-hiding foreign-call entry point that takes ctypes type codes
and recursive layouts plus raw buffers instead of libffi `Type`/`Arg`, to
become the single call path for the VM `_ctypes` (a later change) and other
consumers. Also bring in the `callproc_simple` helper so this host_env copy
stays byte-identical to the pyre-dev copy during the migration.

- `CTypeLayout` (Simple/Pointer/Struct/Union/Array/Opaque) with `size()` and
  an internal libffi-type lowering built on `ffi_type_for_layout`.
- `CallArg` (Typed/Int/Double/Pointer/Aggregate), `CallRet`
  (Void/Code/Pointer/Aggregate), `CallOptions` (use_errno/use_last_error),
  `CallValue` (Void/Scalar/Pointer/Aggregate), and `CallError`.
- `call(addr, &[CallArg], CallRet, CallOptions) -> Result<CallValue,
  CallError>`, built on the existing `Cif`/`ffi_*` primitives: the errno /
  last-error swap wraps only the raw call, and by-value aggregate arguments
  and returns go through `Cif::call_return_into`.
- 16 ABI unit tests over local `extern "C"` functions: scalar parity,
  by-value struct / nested / array-in-struct / {f32,f32} / large-struct
  arguments, small / odd / large struct returns, union size, the errno
  window, and the null-pointer / unknown-code / short-buffer errors.

`callproc` and `callproc_simple` are left in place; both are removed once the
migration onto `call` completes.

Assisted-by: Claude
Replace the VM's direct libffi `Type`/`Arg` marshalling with the unified
host_env `call` entry point. Behavior is preserved on the tested paths:
structs and unions still pass by pointer and struct returns still use the
register-size approximation (passing/returning aggregates by value is a
later change).

- base.rs: `FfiArgValue` → `CArgValue` (Typed{code,bytes}/Int/Double/Pointer)
  with `as_call_arg()`; the paramfunc producers emit `CArgValue`, snapshotting
  buffer bytes at conversion time so no lock is held across the call.
- _ctypes.rs: `CArgObject` carries `value: CArgValue` plus a `keep` slot for
  the from_param z/Z/P null-terminated-copy keepalive that no longer rides in
  the value; `PyCArg` repr reconstructs the scalar for identical output; byref
  updated.
- simple.rs: `to_ffi_value` → `to_carg_value`; from_param emits `CArgValue`
  and its keepalive.
- function.rs: `Argument{value, keep}`; `ArgumentType` yields a `CArgValue`
  (with the "unsupported argument type" check moved up front); `CallInfo`'s
  return fields collapse to `RetSpec` (Void/Pointer/Code) reproducing the exact
  return type and result dispatch; `ctypes_callproc` builds `CallArg`/`CallRet`/
  `CallOptions` and invokes `host_env::ctypes::call`; the errno/last-error swap
  moves into `CallOptions` (the `with_swapped_*` wrappers are removed);
  `convert_raw_result` consumes `CallValue`.
- extra_tests/snippets/stdlib_ctypes_calls.py: libc calls over the live FFI
  path (abs/strlen/sqrt, c_char_p/c_void_p returns, a use_errno round-trip);
  output matches CPython.

Two intentional deltas: a NULL function pointer now raises ValueError instead
of a debug assertion, and a c_char passed positionally without argtypes now
zero-extends via its type code rather than sign-extends via the old tag path,
which also makes it agree with the with-argtypes path.

The old host_env `callproc`/`ffi_*` and `StgInfo`'s libffi field types remain
for now; they are removed when by-value aggregates land.

Assisted-by: Claude
Flip aggregate arguments and returns from the pointer / register-size
approximation to true by-value passing through the host_env `call` entry
point, and carry the driving layouts as host_env `CTypeLayout`.

- base.rs: `StgInfo.ffi_field_types: Vec<FfiType>` becomes
  `field_layouts: Vec<CTypeLayout>`, still built incrementally from the base
  class so struct inheritance is reflected. `StgInfo::to_ffi_type()` is
  replaced by a `type_layout(ty, &stg, vm)` helper that reads a type's own
  layout (aggregates from `field_layouts`, arrays recurse into the element
  type, simple types from their `_type_` code); it takes the already-borrowed
  `StgInfo` so it never re-locks the type. `CArgValue` gains an
  `Aggregate { layout, bytes }` variant lowering to `CallArg::Aggregate`, and
  `struct_union_paramfunc` snapshots the instance bytes into it (tag 'V').
- structure.rs / union.rs: collect each field's `type_layout` into
  `field_layouts` and store it on the finalized `StgInfo`.
- function.rs: `convert_object` passes a struct/union argument by value
  (snapshot bytes + argtype layout); a `byref()` result is still a pointer.
  `RetSpec` gains `Aggregate(CTypeLayout)`; `compute_ret_spec` returns it for
  a struct/union restype; `ctypes_callproc` lowers it to `CallRet::Aggregate`.
  `convert_raw_result` now drops the restype `StgInfo` read guard before
  constructing the result instance: instance construction write-locks the
  type's `StgInfo` to finalize it, which otherwise self-deadlocks against the
  held read guard (latent since the register-size return path, unreachable
  until a struct was actually returned by value).
- _ctypes.rs: a 'V' cparam now reprs as `<cparam 'V' at 0x..>` (the
  object-address default), matching PyCArg_repr; the aggregate `CArgValue`
  routes through that arm.
- extra_tests/snippets/stdlib_ctypes_byvalue.py: div/imaxdiv struct returns
  (8- and 16-byte), inet_ntoa struct and union arguments, with and without
  argtypes; output matches CPython. Also sorts the imports in the sibling
  stdlib_ctypes_calls.py snippet (ruff isort).

`_pack_` / `_swappedbytes_` layouts stay approximate — `CTypeLayout` carries
no explicit field offsets, as the previous libffi field-type path did not
either. host_env is untouched, so both `ctypes.rs` copies stay byte-identical.

test_ctypes is unchanged at run=322 skipped=58.

Assisted-by: Claude
With the VM `_ctypes` and pyre both routed through `call`, delete the
foreign-call paths and libffi-type builders that no longer have a consumer:

- `callproc` (the low-level libffi `Vec<Type>` / `&[Arg]` entry) and its
  `CallResult` return type plus `call_result_bytes`.
- `callproc_simple` and its `SimpleArg` / `SimpleRestype` / `SimpleCallError`
  types (the pointer-only slice added for pyre's first cut), and the
  `lookup_function_symbol_addr_str` helper, along with their tests.
- `ffi_type_for_layout` + `CTypeParamKind`, `ffi_type_from_format`,
  `ffi_type_from_tag`, and `ffi_type_for_return_size` — the aggregate/simple
  libffi-type builders the old marshalling used. `call` and `CTypeLayout`
  build their own libffi types from `ffi_type_from_code` and the small
  `ffi_{pointer,byte_struct,repeat,void,i32,f64}_type` helpers, which stay.

`simple_type_chars` stays (still used by pyre's simple-type validation). The
`call` doc no longer references `callproc_simple`. A stale comment in the VM's
result decoder that named `call_result_bytes` is reworded.

host_env drops from 4051 to 3535 lines; the `call` ABI unit tests remain
(24 tests). test_ctypes is unchanged at run=322 skipped=58.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6bef22e2-a161-4b22-a049-2576dcd4f8a8

📥 Commits

Reviewing files that changed from the base of the PR and between 9ccb216 and 4681347.

📒 Files selected for processing (2)
  • crates/host_env/src/ctypes.rs
  • crates/vm/src/stdlib/_ctypes/function.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/host_env/src/ctypes.rs
  • crates/vm/src/stdlib/_ctypes/function.rs

📝 Walkthrough

Walkthrough

This PR replaces the host_env ctypes libffi call surface with layout-driven call contracts and migrates vm ctypes argument, layout, and result handling to the new CArgValue/CallValue model. It also adds regression snippets for scalar, pointer, errno, and by-value struct/union calls.

Changes

ctypes foreign-call rewrite

Layer / File(s) Summary
Remove legacy call machinery
crates/host_env/src/ctypes.rs
Removes CallResult, call_result_bytes, the older simple-type and layout lowering helpers, and callproc; extends the libffi import with Ret.
Add CTypeLayout and call contracts
crates/host_env/src/ctypes.rs
Adds simple_type_is_pointer, simple_type_chars, CTypeLayout, and the CallArg, CallRet, CallOptions, CallValue, and CallError types.
Implement call() and host tests
crates/host_env/src/ctypes.rs
Introduces call(...) with aggregate validation, errno/last-error swapping, libffi invocation, and result decoding, plus expanded ctypes tests.
CArgObject storage and repr
crates/vm/src/stdlib/_ctypes.rs
CArgObject now stores CArgValue plus keep, repr_str reconstructs FfiValue, and byref() builds CArgValue::pointer.
StgInfo field layouts and type_layout()
crates/vm/src/stdlib/_ctypes/base.rs
Replaces ffi_field_types with field_layouts: Vec<CTypeLayout> and adds type_layout() for simple, array, struct, and union layouts.
Paramfuncs use CArgValue
crates/vm/src/stdlib/_ctypes/base.rs
Simple, array, struct, and union paramfunc paths now produce CArgValue values and aggregate call arguments.
Propagate field layouts
crates/vm/src/stdlib/_ctypes/structure.rs, crates/vm/src/stdlib/_ctypes/union.rs
Structure and union field processing now accumulates field_layouts via type_layout() and stores them on StgInfo.
PyCSimple from_param and to_carg_value
crates/vm/src/stdlib/_ctypes/simple.rs
Simple ctypes conversions now create CArgValue with keep handling, and to_carg_value replaces to_ffi_value.
Argument conversion and build_callargs
crates/vm/src/stdlib/_ctypes/function.rs
Pointer/scalar/aggregate conversion now uses CArgValue, ArgumentType::convert_object, and keep propagation through call-argument construction.
RetSpec and CallValue result flow
crates/vm/src/stdlib/_ctypes/function.rs
Adds RetSpec, replaces ctypes_callproc with a CallValue path, refactors result conversion, and updates PyCFuncPtr::call.
ctypes regression test scripts
extra_tests/snippets/stdlib_ctypes_byvalue.py, extra_tests/snippets/stdlib_ctypes_calls.py
Adds regression coverage for by-value struct/union passing and for scalar, pointer, and errno call behavior.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Python
  participant PyCFuncPtr
  participant ctypes_callproc
  participant host_env_call as host_env::call
  Python->>PyCFuncPtr: call(args)
  PyCFuncPtr->>ctypes_callproc: addr, arguments, RetSpec, CallOptions
  ctypes_callproc->>host_env_call: call(addr, CallArg[], CallRet, CallOptions)
  host_env_call->>host_env_call: lower args, validate sizes, swap errno/last_error
  host_env_call-->>ctypes_callproc: CallValue or CallError
  ctypes_callproc-->>PyCFuncPtr: CallValue / mapped PyErr
  PyCFuncPtr->>PyCFuncPtr: build_result(CallValue)
  PyCFuncPtr-->>Python: converted Python result
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: consolidating ctypes foreign calls into a single host_env call API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`call` read `Code` returns through `low::ffi_arg`. The `libffi_sys`
binding types `ffi_arg` as `c_ulong`, which is 4 bytes under LLP64
(Windows x64), so `low::call` truncated 8-byte returns (`q`/`Q`/`d`) to
the low 4 bytes. `calls_f64_scalar` and `passes_large_struct_by_value`
failed on windows-2025. Read a full register (`u64`) instead;
`decode_type_code` still slices the leading bytes each type code needs.
No change on LP64, where `ffi_arg` is already 8 bytes.

Add `typed_two_scalar_args`, which calls the previously-unused `add_i32`
ABI helper (two-argument scalar path); the helper tripped `-D warnings`
dead-code on clippy.

Assisted-by: Claude
`stdlib_ctypes_byvalue.py` reaches libc through `CDLL(None)` and calls
`div`/`imaxdiv`/`inet_ntoa`, none of which resolve that way on Windows,
so the snippet aborted on windows-2025. Guard it like
`stdlib_ctypes_calls.py`: print "OK" and exit before touching `CDLL`.
The by-value path is covered on Windows by test_ctypes.

Assisted-by: Claude
@youknowone
youknowone marked this pull request as ready for review July 7, 2026 17:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/ctypes.rs`:
- Around line 2124-2155: Guard zero-sized by-value aggregates in the call
lowering path: in the aggregate handling inside the function that builds
`lowered` and `ffi_args`, the current `buffer.len() < expected` check still
allows `expected == 0`, which later causes `Arg::new(&buffer[0])` to panic.
Update the `CallArg::Aggregate` branch and/or the `ffi_args` construction to
reject or specially handle zero-sized `Structure`/`Union` arguments before
indexing into the buffer.

In `@crates/vm/src/stdlib/_ctypes/function.rs`:
- Around line 173-180: The default Python int to i32 conversion in the argument
conversion logic is silently truncating overflow to 0; update the conversion in
the function that builds Argument/CArgValue::Int so it checks the bigint-to-i32
result and raises OverflowError on out-of-range values instead of using
unwrap_or(0). Keep the behavior aligned with convert_to_pointer in the same
module by preserving the original error path rather than coercing invalid
inputs.
- Around line 270-285: The by-value struct/union handling in function.rs
currently copies only the raw bytes from the converted PyCData, which can drop
ownership of nested backing references too early. Update the aggregate path in
the conversion logic to keep the converted object alive by returning it
alongside the CArgValue, specifically in the PyCStructure/PyCUnion branch of the
argument conversion code. Preserve the existing byte snapshot and layout
selection, but return Some(converted.clone()) with the aggregate so any
_objects/kept_refs remain valid through the FFI call.
- Around line 211-241: Update ArgumentType for PyTypeRef in convert_object to
recognize PyCArray before the generic _type_ string validation, since array
argtypes carry an element type object in _type_ rather than a ctypes code
string. Adjust the type-check branch around the fast_issubclass and
get_attr("_type_", vm) logic so arrays are accepted and can still be decayed
through from_param into a pointer, instead of failing the PyStr downcast for
cases like c_int * 3.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: c9d5518f-8def-4b3e-9aa5-6aaff2b2ab1f

📥 Commits

Reviewing files that changed from the base of the PR and between 3f33073 and 9ccb216.

📒 Files selected for processing (9)
  • crates/host_env/src/ctypes.rs
  • crates/vm/src/stdlib/_ctypes.rs
  • crates/vm/src/stdlib/_ctypes/base.rs
  • crates/vm/src/stdlib/_ctypes/function.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/stdlib/_ctypes/structure.rs
  • crates/vm/src/stdlib/_ctypes/union.rs
  • extra_tests/snippets/stdlib_ctypes_byvalue.py
  • extra_tests/snippets/stdlib_ctypes_calls.py

Comment thread crates/host_env/src/ctypes.rs
Comment on lines 173 to 180
// 10. Python int -> i32 (default integer type)
if let Ok(int_val) = value.try_int(vm) {
let val = int_val.as_bigint().to_i32().unwrap_or(0);
return Ok(Argument {
ffi_type: ffi_i32_type(),
keep: None,
value: FfiArgValue::Scalar(FfiValue::I32(val)),
value: CArgValue::Int(val),
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Silent truncation on integer overflow instead of raising OverflowError.

to_i32().unwrap_or(0) silently maps an out-of-range Python int to 0 rather than surfacing an error. This is inconsistent with convert_to_pointer in the same file (lines 108-109), which explicitly raises OverflowError when a value doesn't fit. CPython's ConvParam for the no-argtypes default-int case similarly propagates OverflowError on overflow rather than truncating. Silently coercing to 0 produces a wrong value passed to the foreign function without any diagnostic.

🐛 Proposed fix
-    if let Ok(int_val) = value.try_int(vm) {
-        let val = int_val.as_bigint().to_i32().unwrap_or(0);
-        return Ok(Argument {
-            keep: None,
-            value: CArgValue::Int(val),
-        });
-    }
+    if let Ok(int_val) = value.try_int(vm) {
+        let val = int_val
+            .as_bigint()
+            .to_i32()
+            .ok_or_else(|| vm.new_overflow_error("int too large to convert"))?;
+        return Ok(Argument {
+            keep: None,
+            value: CArgValue::Int(val),
+        });
+    }
📝 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
// 10. Python int -> i32 (default integer type)
if let Ok(int_val) = value.try_int(vm) {
let val = int_val.as_bigint().to_i32().unwrap_or(0);
return Ok(Argument {
ffi_type: ffi_i32_type(),
keep: None,
value: FfiArgValue::Scalar(FfiValue::I32(val)),
value: CArgValue::Int(val),
});
}
// 10. Python int -> i32 (default integer type)
if let Ok(int_val) = value.try_int(vm) {
let val = int_val
.as_bigint()
.to_i32()
.ok_or_else(|| vm.new_overflow_error("int too large to convert"))?;
return Ok(Argument {
keep: None,
value: CArgValue::Int(val),
});
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/src/stdlib/_ctypes/function.rs` around lines 173 - 180, The default
Python int to i32 conversion in the argument conversion logic is silently
truncating overflow to 0; update the conversion in the function that builds
Argument/CArgValue::Int so it checks the bigint-to-i32 result and raises
OverflowError on out-of-range values instead of using unwrap_or(0). Keep the
behavior aligned with convert_to_pointer in the same module by preserving the
original error path rather than coercing invalid inputs.

Comment on lines 211 to +241
impl ArgumentType for PyTypeRef {
fn to_ffi_type(&self, vm: &VirtualMachine) -> PyResult<FfiType> {
use super::pointer::PyCPointer;
use super::structure::PyCStructure;

// CArgObject (from byref()) should be treated as pointer
if self.fast_issubclass(CArgObject::static_type()) {
return Ok(ffi_pointer_type());
}

// Pointer types (POINTER(T)) are always pointer FFI type
// Check if type is a subclass of _Pointer (PyCPointer)
if self.fast_issubclass(PyCPointer::static_type()) {
return Ok(ffi_pointer_type());
}

// Structure types are passed as pointers
if self.fast_issubclass(PyCStructure::static_type()) {
return Ok(ffi_pointer_type());
}
fn convert_object(
&self,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<(CArgValue, Option<PyObjectRef>)> {
// Validate the argument type up front (mirrors the pre-conversion
// check): pointer-like ctypes types are always acceptable; a simple
// type must carry a known _type_ code; anything else is unsupported.
let type_code = if self.fast_issubclass(CArgObject::static_type())
|| self.fast_issubclass(PyCPointer::static_type())
|| self.fast_issubclass(PyCStructure::static_type())
|| self.fast_issubclass(PyCUnion::static_type())
{
None
} else {
// Use get_attr to traverse MRO (for subclasses like MyInt(c_int))
let typ = self
.as_object()
.get_attr(vm.ctx.intern_str("_type_"), vm)
.ok()
.ok_or_else(|| vm.new_type_error("Unsupported argument type"))?;
let typ = typ
.downcast_ref::<PyStr>()
.ok_or_else(|| vm.new_type_error("Unsupported argument type"))?
.to_string();
if ffi_type_from_code(&typ).is_none() {
return Err(vm.new_type_error(format!("Unsupported argument type: {typ}")));
}
Some(typ)
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and nearby symbols.
git ls-files crates/vm/src/stdlib/_ctypes/function.rs crates/vm/src/stdlib/_ctypes/base.rs crates/vm/src/stdlib/_ctypes/*.rs | sed -n '1,120p'

printf '\n== outline function.rs ==\n'
ast-grep outline crates/vm/src/stdlib/_ctypes/function.rs --view expanded || true

printf '\n== outline base.rs ==\n'
ast-grep outline crates/vm/src/stdlib/_ctypes/base.rs --view expanded || true

printf '\n== search for PyCArray and convert_to_pointer usage ==\n'
rg -n "PyCArray|convert_to_pointer|array_paramfunc|call_paramfunc|from_param|Unsupported argument type" crates/vm/src/stdlib/_ctypes -S

Repository: RustPython/RustPython

Length of output: 23436


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read only the relevant slices around the claimed code paths.
sed -n '1,220p' crates/vm/src/stdlib/_ctypes/base.rs
printf '\n--- function.rs slice ---\n'
sed -n '1,320p' crates/vm/src/stdlib/_ctypes/function.rs

Repository: RustPython/RustPython

Length of output: 20130


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the exact type conversion branches in function.rs.
nl -ba crates/vm/src/stdlib/_ctypes/function.rs | sed -n '180,280p'

Repository: RustPython/RustPython

Length of output: 199


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read array type definitions and the relevant conversion helpers.
nl -ba crates/vm/src/stdlib/_ctypes/base.rs | sed -n '1,260p'

Repository: RustPython/RustPython

Length of output: 199


🏁 Script executed:

#!/bin/bash
set -euo pipefail

set -o pipefail

printf '== function.rs relevant slice ==\n'
nl -ba crates/vm/src/stdlib/_ctypes/function.rs | sed -n '200,270p'

printf '\n== base.rs relevant slice around PyCArray and helpers ==\n'
rg -n "struct PyCArray|impl ArgumentType|array_paramfunc|convert_to_pointer|_type_|from_param|call_paramfunc" crates/vm/src/stdlib/_ctypes/base.rs crates/vm/src/stdlib/_ctypes/function.rs -n -S

Repository: RustPython/RustPython

Length of output: 232


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== array.rs around type construction and from_param ==\n'
sed -n '1,220p' crates/vm/src/stdlib/_ctypes/array.rs

printf '\n== array.rs around PyCArrayType.from_param ==\n'
sed -n '300,360p' crates/vm/src/stdlib/_ctypes/array.rs

printf '\n== array.rs around PyCArray definition ==\n'
sed -n '390,470p' crates/vm/src/stdlib/_ctypes/array.rs

printf '\n== search for _type_ assignment/access in _ctypes ==\n'
rg -n '"_type_"|_type_' crates/vm/src/stdlib/_ctypes/array.rs crates/vm/src/stdlib/_ctypes/base.rs crates/vm/src/stdlib/_ctypes/simple.rs crates/vm/src/stdlib/_ctypes/pointer.rs crates/vm/src/stdlib/_ctypes/structure.rs crates/vm/src/stdlib/_ctypes/union.rs -S

Repository: RustPython/RustPython

Length of output: 48584


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read only the relevant array/pointer helper slices with portable tools.
sed -n '120,220p' crates/vm/src/stdlib/_ctypes/array.rs
printf '\n---\n'
sed -n '308,350p' crates/vm/src/stdlib/_ctypes/array.rs
printf '\n---\n'
sed -n '390,460p' crates/vm/src/stdlib/_ctypes/array.rs
printf '\n---\n'
rg -n '_type_' crates/vm/src/stdlib/_ctypes/array.rs crates/vm/src/stdlib/_ctypes/base.rs crates/vm/src/stdlib/_ctypes/simple.rs crates/vm/src/stdlib/_ctypes/pointer.rs crates/vm/src/stdlib/_ctypes/structure.rs crates/vm/src/stdlib/_ctypes/union.rs -S

Repository: RustPython/RustPython

Length of output: 43119


Handle array argtypes before _type_ validation. PyCArray types store their element type in _type_, so self.as_object().get_attr("_type_", vm) returns a type object here, not a string code. downcast_ref::<PyStr>() then fails and rejects entries like argtypes = [c_int * 3] before from_param can decay them to a pointer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/src/stdlib/_ctypes/function.rs` around lines 211 - 241, Update
ArgumentType for PyTypeRef in convert_object to recognize PyCArray before the
generic _type_ string validation, since array argtypes carry an element type
object in _type_ rather than a ctypes code string. Adjust the type-check branch
around the fast_issubclass and get_attr("_type_", vm) logic so arrays are
accepted and can still be decayed through from_param into a pointer, instead of
failing the PyStr downcast for cases like c_int * 3.

Comment thread crates/vm/src/stdlib/_ctypes/function.rs
The by-value struct/union argument path snapshotted the instance bytes
and returned `None` for the keep-alive slot, dropping the `from_param`
result before the foreign call. When the snapshot embeds pointers into
buffers owned by that object's keep-alive set, the call could read freed
memory. Return the converted instance as the keep-alive owner.

Assisted-by: Claude
A zero-sized `Structure`/`Union` argument reaches the aggregate lowering
with `layout.size() == 0` and an empty buffer, which passed the
`buffer.len() < expected` check and then panicked on `&buffer[0]`. Use
`buffer.first().unwrap_or(&0u8)`; libffi reads nothing for a zero-size
type.

Assisted-by: Claude
@youknowone
youknowone merged commit be384a3 into RustPython:main Jul 8, 2026
26 checks passed
@youknowone
youknowone deleted the hostenv-ctypes-call branch July 8, 2026 03:42
@coderabbitai coderabbitai Bot mentioned this pull request Jul 9, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant