Skip to content
Merged
Changes from all commits
Commits
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
125 changes: 115 additions & 10 deletions crates/capi/src/listobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use crate::object::define_py_check;
use crate::pystate::with_vm;
use core::ffi::c_int;
use core::ptr::NonNull;
use rustpython_vm::AsObject;
use rustpython_vm::PyObjectRef;
use rustpython_vm::builtins::PyList;
use rustpython_vm::sliceable::{SaturatedSlice, SliceableSequenceMutOp, SliceableSequenceOp};

define_py_check!(fn PyList_Check, types.list_type);
define_py_check!(exact fn PyList_CheckExact, types.list_type);
Expand Down Expand Up @@ -110,16 +112,69 @@ pub unsafe extern "C" fn PyList_Reverse(list: *mut PyObject) -> c_int {
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyList_AsTuple(list: *mut PyObject) -> *mut PyObject {
with_vm(|vm| {
let list = unsafe { &*list }.try_downcast_ref::<PyList>(vm)?;
Ok(vm.ctx.new_tuple(list.borrow_vec().to_vec()))
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyList_GetSlice(
list: *mut PyObject,
low: isize,
high: isize,
) -> *mut PyObject {
with_vm(|vm| {
let list = unsafe { &*list }.try_downcast_ref::<PyList>(vm)?;
let vec = list.borrow_vec();
let sliced = vec.getitem_by_slice(vm, SaturatedSlice::from_parts(low, high, 1))?;
Ok(vm.ctx.new_list(sliced))
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyList_SetSlice(
list: *mut PyObject,
low: isize,
high: isize,
itemlist: *mut PyObject,
) -> c_int {
with_vm(|vm| {
let list = unsafe { &*list }.try_downcast_ref::<PyList>(vm)?;
let slice = SaturatedSlice::from_parts(low, high, 1);
let mut vec = list.borrow_vec_mut();

if itemlist.is_null() {
vec.delitem_by_slice(vm, slice)?;
return Ok(());
}

let items: Vec<PyObjectRef> = unsafe { &*itemlist }.try_to_value(vm)?;
vec.setitem_by_slice(vm, slice, &items)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyList_Sort(list: *mut PyObject) -> c_int {
with_vm(|vm| {
let list = unsafe { &*list }.try_downcast_ref::<PyList>(vm)?;
vm.call_method(list.as_object(), "sort", ())?;
Ok(())
Comment on lines +160 to +164

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the implementation and inspect surrounding code
FILE="crates/capi/src/listobject.rs"
nl -ba "$FILE" | sed -n '130,230p'

# Inspect how list sorting is implemented on the RustPython side
rg -n "fn\s+sort|class.*sort|PyList_.*Sort|\"sort\"" crates/capi/src/listobject.rs crates/vm crates/payload crates | head -n 200

# Find the PyList definition and its sort method implementation
rg -n "struct\s+PyList|impl\s+PyList|PyPayload.*PyList|pymethod.*sort|fn\s+sort\(" crates | head -n 200

# Check what try_downcast_ref::<PyList> does / whether it accepts subclasses
rg -n "try_downcast_ref<.*PyList|try_downcast_ref" crates/vm crates/payload crates -S | head -n 200

Repository: RustPython/RustPython

Length of output: 108


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="crates/capi/src/listobject.rs"

# Show the relevant section with line numbers (cat -n)
if [ -f "$FILE" ]; then
  echo "==== $FILE (lines 130-220) ===="
  sed -n '130,220p' "$FILE" | cat -n
else
  echo "Missing file: $FILE"
  exit 1
fi

echo "==== Search: PyList_Sort and call_method(\"sort\") ===="
rg -n "PyList_Sort|call_method\(.*\"sort\"|\"sort\"\s*,\s*\(\s*\)" crates/capi/src/listobject.rs crates/capi/src -S

echo "==== Search: list sort implementation (PyList::sort / method sort) ===="
rg -n "impl\s+PyList|fn\s+sort\(|pymethod.*sort|method.*sort|\"sort\"" crates -S | head -n 200

echo "==== Search: try_downcast_ref::<PyList> usage ===="
rg -n "try_downcast_ref::<\s*PyList\s*>" crates -S

Repository: RustPython/RustPython

Length of output: 6883


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect try_downcast_ref implementation/semantics
rg -n "try_downcast_ref::<\s*PyList\s*>|try_downcast_ref<|fn\s+try_downcast_ref" crates -S | head -n 200

# Try to find the concrete trait/method definition (more targeted)
rg -n "try_downcast_ref\s*\(" crates/vm crates/common crates/core crates -S | head -n 200

Repository: RustPython/RustPython

Length of output: 1249


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="crates/vm/src/object/core.rs"

echo "=== try_downcast_ref in $FILE (around line ~1549) ==="
sed -n '1520,1605p' "$FILE" | cat -n

echo "=== Search for PyPayload / downcast logic around try_downcast_ref ==="
rg -n "trait\s+PyPayload|downcast|payload_type_id|TypeId|is_a|downcast_ref" crates/vm/src/object/core.rs crates/vm/src/object -S | head -n 200

Repository: RustPython/RustPython

Length of output: 12850


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect PyPayload::validate_downcastable_from default + semantics
FILE="crates/vm/src/object/payload.rs"
echo "=== $FILE (relevant section) ==="
sed -n '1,120p' "$FILE" | cat -n

# Inspect PyList payload/type implementation to see what validates downcastable
FILE="crates/vm/src/builtins/list.rs"
echo "=== $FILE: search PyPayload impl for PyList ==="
rg -n "PyPayload\s+for\s+PyList|impl\s+PyPayload\s+for\s+PyList|PAYLOAD_TYPE_ID|validate_downcastable_from" "$FILE" crates/vm/src/builtins -S | head -n 200

echo "=== $FILE: show around PyList impl blocks near sort ==="
rg -n "struct\s+PyList|impl\s+PyList|fn\s+sort\(" "$FILE" -S

Repository: RustPython/RustPython

Length of output: 5750


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="crates/vm/src/builtins/list.rs"

# Show PyPayload impl for PyList (near line 84)
sed -n '60,120p' "$FILE" | cat -n

# Show more of the impl to include validation/downcast details (if any)
sed -n '120,220p' "$FILE" | cat -n

# Show the PyList sort implementation around sort() method
sed -n '350,470p' "$FILE" | cat -n

Repository: RustPython/RustPython

Length of output: 10897


🌐 Web query:

CPython PyList_Sort implementation listobject.c calls overridden sort subclass

💡 Result:

In CPython, the list.sort() method is implemented in Objects/listobject.c using a highly optimized C function, list_sort_impl (which corresponds to the list.sort operation). This implementation does not call overridden sort methods defined in subclasses of list [1][2][3]. The internal C implementation treats list subclasses as standard list objects for the purposes of sorting [1][3]. When list.sort() is called on an instance of a list subclass, the C code directly manipulates the underlying data structure of the object—effectively ignoring any custom sort logic defined in the Python layer of the subclass [1][2][3]. This is a documented behavior pattern in CPython where many C-implemented methods for built-in types (like list, dict, and heapq operations) operate on the concrete C structure and bypass Python-level overrides [4][1][2][3]. The implementation utilizes an adaptive, stable, natural mergesort algorithm (often referred to as Timsort), designed for efficiency by minimizing comparisons, which are expensive in CPython [5][6][7]. Because the implementation relies on direct memory access and low-level C functions to handle the sorting logic, it does not perform the standard Python method resolution order (MRO) lookups that would be required to detect and execute a subclass-overridden sort method [1][3]. Consequently, if a user overrides sort in a list subclass, calling .sort() on an instance of that class will still execute the native C implementation, not the custom Python code [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find call_method implementation
rg -n "fn\s+call_method\s*\(" crates/vm -S | head -n 50

# Also search for the string-based call path
rg -n "call_method\(.*\"sort\"" crates/vm crates/capi -S | head -n 50

# Inspect call_method internals (where it does attribute lookup / descriptor resolution)
# Try to locate the specific method that `call_method` delegates to.
rg -n "call_method\(|call_method_with|" crates/vm -S | head -n 200

# Check for helpers used by call_method for attribute resolution by name
rg -n "get_attr|lookup_attr|slot|descriptor|MRO|resolve" crates/vm/src -S | head -n 200

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Locate the definition of call_method
rg -n "call_method" crates/vm/src -S | head -n 200

# 2) Look specifically for the signature text "pub .*call_method" / "fn .*call_method"
rg -n "pub\s+.*call_method|fn\s+.*call_method" crates/vm/src -S | head -n 200

# 3) Find where call_method resolves attributes by name ("sort" in our case will be passed as a string)
rg -n "call_method.*lookup|lookup.*call_method|resolve.*call_method|get_attr|lookup_attr|descriptor|MRO" crates/vm/src -S | head -n 200

Repository: RustPython/RustPython

Length of output: 38370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="crates/vm/src/vm/vm_object.rs"
sed -n '90,210p' "$FILE" | cat -n

# Also inspect get_attribute/get_attr helpers used by call_method
rg -n "get_attr_inner|get_attribute_opt|call_if_get_descriptor|call_get_descriptor|method_name" crates/vm/src/vm/vm_object.rs crates/vm/src/vm/mod.rs crates/vm/src/vm/method.rs -S | head -n 200

Repository: RustPython/RustPython

Length of output: 6400


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find PyMethod::get implementation
rg -n "struct\s+PyMethod|impl\s+PyMethod|fn\s+get\s*\(" crates/vm/src/vm/method.rs crates/vm/src -S | head -n 200

# Show the relevant portion around PyMethod::get / attribute resolution
sed -n '1,220p' crates/vm/src/vm/method.rs | cat -n

Repository: RustPython/RustPython

Length of output: 8851


Avoid Python-level method dispatch in PyList_Sort.

PyList_Sort downcasts to PyList and then calls vm.call_method(list.as_object(), "sort", ()), which resolves "sort" via PyMethod::get against the object’s runtime class and instance __dict__. That allows a list subclass override (or an instance attribute named sort) to run user code, unlike CPython’s PyList_Sort behavior. Resolve/invoke the sort implementation from the exact built-in list type (or call the internal list sort routine) instead of generic call_method.

🤖 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/capi/src/listobject.rs` around lines 160 - 164, PyList_Sort currently
downcasts to PyList and uses vm.call_method(list.as_object(), "sort", ()) which
performs normal Python-level dispatch and can run overridden user code; change
it to invoke the builtin list sort implementation directly (not via instance
dispatch). Specifically, replace vm.call_method(...) in PyList_Sort with a
direct call to the internal list sorting routine (the in-place sorter for
PyList) or call the "sort" function bound to the exact built-in list type rather
than the instance (e.g., resolve the attribute on vm.ctx.types.list_type and
invoke that bound function with the list as self, or call PyList's internal sort
method such as the library's sort_inplace routine), ensuring no instance-level
__dict__/subclass override can run. Use the symbols PyList_Sort, PyList, and
avoid vm.call_method to implement this change.

})
}

#[cfg(false)]
mod tests {
use pyo3::exceptions::PyIndexError;
use pyo3::prelude::*;
use pyo3::types::PyList;
use pyo3::types::{PyList, PyListMethods};

#[test]
fn test_create_list() {
fn create_list() {
Python::attach(|py| {
let list = PyList::new(py, &[1, 2, 3]).unwrap();
let list = PyList::new(py, [1, 2, 3]).unwrap();
assert_eq!(list.len(), 3);
assert_eq!(list.get_item(0).unwrap().extract::<u32>().unwrap(), 1);
assert_eq!(list.get_item(1).unwrap().extract::<u32>().unwrap(), 2);
Expand All @@ -129,9 +184,9 @@ mod tests {
}

#[test]
fn test_replace_item_in_list() {
fn replace_item_in_list() {
Python::attach(|py| {
let list = PyList::new(py, &[1]).unwrap();
let list = PyList::new(py, [1]).unwrap();
assert_eq!(list.len(), 1);
list.set_item(0, 2).unwrap();
assert_eq!(list.len(), 1);
Expand All @@ -140,7 +195,7 @@ mod tests {
}

#[test]
fn test_set_item_out_of_range() {
fn set_item_out_of_range() {
Python::attach(|py| {
let list = PyList::empty(py);
assert!(
Expand All @@ -152,7 +207,7 @@ mod tests {
}

#[test]
fn test_list_append() {
fn list_append() {
Python::attach(|py| {
let list = PyList::empty(py);
assert_eq!(list.len(), 0);
Expand All @@ -163,7 +218,7 @@ mod tests {
}

#[test]
fn test_list_insert() {
fn list_insert() {
Python::attach(|py| {
let list = PyList::empty(py);
assert_eq!(list.len(), 0);
Expand All @@ -175,12 +230,62 @@ mod tests {
}

#[test]
fn test_list_reverse() {
fn list_reverse() {
Python::attach(|py| {
let list = PyList::new(py, &[1, 2, 3]).unwrap();
let list = PyList::new(py, [1, 2, 3]).unwrap();
list.reverse().unwrap();
assert_eq!(list.get_item(0).unwrap().extract::<u32>().unwrap(), 3);
assert_eq!(list.get_item(2).unwrap().extract::<u32>().unwrap(), 1);
})
}

#[test]
fn list_as_tuple() {
Python::attach(|py| {
let list = PyList::new(py, [1, 2, 3]).unwrap();
let tuple = list.to_tuple();
assert_eq!(tuple.len(), 3);
assert_eq!(tuple.get_item(0).unwrap().extract::<u32>().unwrap(), 1);

list.set_item(0, 9).unwrap();
assert_eq!(tuple.get_item(0).unwrap().extract::<u32>().unwrap(), 1);
})
}

#[test]
fn list_get_slice() {
Python::attach(|py| {
let list = PyList::new(py, [1, 2, 3, 4]).unwrap();
let slice = list.get_slice(1, 10);
assert_eq!(slice.len(), 3);
assert_eq!(slice.get_item(0).unwrap().extract::<u32>().unwrap(), 2);
assert_eq!(slice.get_item(2).unwrap().extract::<u32>().unwrap(), 4);
})
}

#[test]
fn list_set_slice() {
Python::attach(|py| {
let list = PyList::new(py, [1, 2, 3, 4]).unwrap();
let repl = PyList::new(py, [8, 9]).unwrap();
list.set_slice(1, 3, repl.as_any()).unwrap();

assert_eq!(list.len(), 4);
assert_eq!(list.get_item(0).unwrap().extract::<u32>().unwrap(), 1);
assert_eq!(list.get_item(1).unwrap().extract::<u32>().unwrap(), 8);
assert_eq!(list.get_item(2).unwrap().extract::<u32>().unwrap(), 9);
assert_eq!(list.get_item(3).unwrap().extract::<u32>().unwrap(), 4);
})
}

#[test]
fn list_sort() {
Python::attach(|py| {
let list = PyList::new(py, [3, 1, 2]).unwrap();
list.sort().unwrap();
assert_eq!(list.get_item(0).unwrap().extract::<u32>().unwrap(), 1);
assert_eq!(list.get_item(1).unwrap().extract::<u32>().unwrap(), 2);
assert_eq!(list.get_item(2).unwrap().extract::<u32>().unwrap(), 3);
})
}
}
Loading