luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix soundness issues w/r `ExactSizeIterator`

This commit is contained in:
mejrs 2022-01-24 21:14:54 +01:00
parent d1542a8a79
commit d53a985f79
2 changed files with 431 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

202
src/types/list.rs
View File

@ -2,12 +2,14 @@
//
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython
use std::convert::TryInto;
use crate::err::{self, PyResult};
use crate::ffi::{self, Py_ssize_t};
use crate::internal_tricks::get_ssize_index;
use crate::types::PySequence;
use crate::{
AsPyPointer, IntoPy, IntoPyPointer, PyAny, PyObject, PyTryFrom, Python, ToBorrowedObject,
AsPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, PyTryFrom, Python, ToBorrowedObject,
ToPyObject,
};
@ -18,30 +20,79 @@ pub struct PyList(PyAny);
pyobject_native_type_core!(PyList, ffi::PyList_Type, #checkfunction=ffi::PyList_Check);
#[inline]
unsafe fn new_from_iter<T>(
elements: impl ExactSizeIterator<Item = T>,
#[track_caller]
fn new_from_iter<T>(
py: Python,
mut elements: impl ExactSizeIterator<Item = T>,
convert: impl Fn(T) -> PyObject,
) -> *mut ffi::PyObject {
let ptr = ffi::PyList_New(elements.len() as Py_ssize_t);
for (i, e) in elements.enumerate() {
let obj = convert(e).into_ptr();
#[cfg(not(Py_LIMITED_API))]
ffi::PyList_SET_ITEM(ptr, i as Py_ssize_t, obj);
#[cfg(Py_LIMITED_API)]
ffi::PyList_SetItem(ptr, i as Py_ssize_t, obj);
unsafe {
// PyList_New checks for overflow but has a bad error message, so we check ourselves
let len: Py_ssize_t = elements.len().try_into().unwrap_or_else(|_| {
panic!("out of range integral type conversion attempted on `elements.len()`")
});
let list = ffi::PyList_New(len);
// - Panics if the ptr is null
// - Cleans up the list if `convert` or the asserts panic
let guard: Py<PyAny> = Py::from_owned_ptr(py, list);
let mut counter: Py_ssize_t = 0;
for (i, e) in elements.by_ref().enumerate().take(len as usize) {
let obj = convert(e).into_ptr();
#[cfg(not(Py_LIMITED_API))]
ffi::PyList_SET_ITEM(list, i as Py_ssize_t, obj);
#[cfg(Py_LIMITED_API)]
ffi::PyList_SetItem(list, i as Py_ssize_t, obj);
counter += 1;
}
assert!(elements.next().is_none(), "Attempted to create PyList but `elements` was larger than reported by its `ExactSizeIterator` implementation.");
assert_eq!(len, counter, "Attempted to create PyList but `elements` was smaller than reported by its `ExactSizeIterator` implementation.");
std::mem::forget(guard);
list
}
ptr
}
impl PyList {
/// Constructs a new list with the given elements.
///
/// If you want to create a [`PyList`] with elements of different or unknown types, or from an
/// iterable that doesn't implement [`ExactSizeIterator`], use [`PyList::append`].
///
/// # Examples
///
/// ```rust
/// use pyo3::prelude::*;
/// use pyo3::types::PyList;
///
/// # fn main() {
/// Python::with_gil(|py| {
/// let elements: Vec<i32> = vec![0, 1, 2, 3, 4, 5];
/// let list: &PyList = PyList::new(py, elements);
/// assert_eq!(format!("{:?}", list), "[0, 1, 2, 3, 4, 5]");
/// });
/// # }
/// ```
///
/// # Panics
///
/// This function will panic if `element`'s [`ExactSizeIterator`] implementation is incorrect.
/// All standard library structures implement this trait correctly, if they do, so calling this
/// function with (for example) [`Vec`]`<T>` or `&[T]` will always succeed.
#[track_caller]
pub fn new<T, U>(py: Python<'_>, elements: impl IntoIterator<Item = T, IntoIter = U>) -> &PyList
where
T: ToPyObject,
U: ExactSizeIterator<Item = T>,
{
unsafe {
py.from_owned_ptr::<PyList>(new_from_iter(elements.into_iter(), |e| e.to_object(py)))
py.from_owned_ptr::<PyList>(new_from_iter(py, elements.into_iter(), |e| {
e.to_object(py)
}))
}
}
@ -288,7 +339,7 @@ where
T: ToPyObject,
{
fn to_object(&self, py: Python<'_>) -> PyObject {
unsafe { PyObject::from_owned_ptr(py, new_from_iter(self.iter(), |e| e.to_object(py))) }
unsafe { PyObject::from_owned_ptr(py, new_from_iter(py, self.iter(), |e| e.to_object(py))) }
}
}
@ -306,7 +357,9 @@ where
T: IntoPy<PyObject>,
{
fn into_py(self, py: Python) -> PyObject {
unsafe { PyObject::from_owned_ptr(py, new_from_iter(self.into_iter(), |e| e.into_py(py))) }
unsafe {
PyObject::from_owned_ptr(py, new_from_iter(py, self.into_iter(), |e| e.into_py(py)))
}
}
}
@ -314,7 +367,7 @@ where
mod tests {
use crate::types::PyList;
use crate::Python;
use crate::{IntoPy, PyObject, PyTryFrom, ToPyObject};
use crate::{IntoPy, Py, PyAny, PyObject, PyTryFrom, ToPyObject};
#[test]
fn test_new() {
@ -720,4 +773,123 @@ mod tests {
assert!(list.index(42i32).is_err());
});
}
use std::ops::Range;
struct FaultyIter(Range<usize>, usize);
impl Iterator for FaultyIter {
type Item = usize;
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}
}
impl ExactSizeIterator for FaultyIter {
fn len(&self) -> usize {
self.1
}
}
#[test]
#[should_panic(
expected = "Attempted to create PyList but `elements` was larger than reported by its `ExactSizeIterator` implementation."
)]
fn too_long_iterator() {
Python::with_gil(|py| {
let iter = FaultyIter(0..usize::MAX, 73);
let _list = PyList::new(py, iter);
})
}
#[test]
#[should_panic(
expected = "Attempted to create PyList but `elements` was smaller than reported by its `ExactSizeIterator` implementation."
)]
fn too_short_iterator() {
Python::with_gil(|py| {
let iter = FaultyIter(0..35, 73);
let _list = PyList::new(py, iter);
})
}
#[test]
#[should_panic(
expected = "out of range integral type conversion attempted on `elements.len()`"
)]
fn overflowing_size() {
Python::with_gil(|py| {
let iter = FaultyIter(0..0, usize::MAX);
let _list = PyList::new(py, iter);
})
}
#[test]
fn bad_clone_mem_leaks() {
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0);
#[crate::pyclass]
#[pyo3(crate = "crate")]
struct Bad(usize);
impl Clone for Bad {
fn clone(&self) -> Self {
if self.0 == 42 {
// This panic should not lead to a memory leak
panic!()
};
NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst);
Bad(self.0)
}
}
impl Drop for Bad {
fn drop(&mut self) {
NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst);
}
}
impl ToPyObject for Bad {
fn to_object(&self, py: Python) -> Py<PyAny> {
self.to_owned().into_py(py)
}
}
struct FaultyIter(Range<usize>, usize);
impl Iterator for FaultyIter {
type Item = Bad;
fn next(&mut self) -> Option<Self::Item> {
self.0.next().map(|i| {
NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst);
Bad(i)
})
}
}
impl ExactSizeIterator for FaultyIter {
fn len(&self) -> usize {
self.1
}
}
Python::with_gil(|py| {
let _ = std::panic::catch_unwind(|| {
let iter = FaultyIter(0..50, 50);
let _list = PyList::new(py, iter);
});
});
assert_eq!(
NEEDS_DESTRUCTING_COUNT.load(SeqCst),
0,
"Some destructors did not run"
);
}
}

260
src/types/tuple.rs
View File

@ -1,5 +1,7 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
use std::convert::TryInto;
use crate::ffi::{self, Py_ssize_t};
use crate::internal_tricks::get_ssize_index;
use crate::types::PySequence;
@ -8,6 +10,44 @@ use crate::{
PyResult, PyTryFrom, Python, ToBorrowedObject, ToPyObject,
};
#[inline]
#[track_caller]
fn new_from_iter<T>(
py: Python,
mut elements: impl ExactSizeIterator<Item = T>,
convert: impl Fn(T) -> PyObject,
) -> *mut ffi::PyObject {
unsafe {
// PyTuple_New checks for overflow but has a bad error message, so we check ourselves
let len: Py_ssize_t = elements.len().try_into().unwrap_or_else(|_| {
panic!("out of range integral type conversion attempted on `elements.len()`")
});
let tup = ffi::PyTuple_New(len);
// - Panics if the ptr is null
// - Cleans up the tuple if `convert` or the asserts panic
let guard: Py<PyAny> = Py::from_owned_ptr(py, tup);
let mut counter: Py_ssize_t = 0;
for (i, e) in elements.by_ref().enumerate().take(len as usize) {
let obj = convert(e).into_ptr();
#[cfg(not(Py_LIMITED_API))]
ffi::PyTuple_SET_ITEM(tup, i as Py_ssize_t, obj);
#[cfg(Py_LIMITED_API)]
ffi::PyTuple_SetItem(tup, i as Py_ssize_t, obj);
counter += 1;
}
assert!(elements.next().is_none(), "Attempted to create PyTuple but `elements` was larger than reported by its `ExactSizeIterator` implementation.");
assert_eq!(len, counter, "Attempted to create PyTuple but `elements` was smaller than reported by its `ExactSizeIterator` implementation.");
std::mem::forget(guard);
tup
}
}
/// Represents a Python `tuple` object.
///
/// This type is immutable.
@ -18,23 +58,40 @@ pyobject_native_type_core!(PyTuple, ffi::PyTuple_Type, #checkfunction=ffi::PyTup
impl PyTuple {
/// Constructs a new tuple with the given elements.
///
/// If you want to create a [`PyTuple`] with elements of different or unknown types, or from an
/// iterable that doesn't implement [`ExactSizeIterator`], create a Rust tuple with the given
/// elements and convert it at once.
///
/// # Examples
///
/// ```rust
/// use pyo3::prelude::*;
/// use pyo3::types::PyTuple;
///
/// # fn main() {
/// Python::with_gil(|py| {
/// let elements: Vec<i32> = vec![0, 1, 2, 3, 4, 5];
/// let tuple: &PyTuple = PyTuple::new(py, elements);
/// assert_eq!(format!("{:?}", tuple), "(0, 1, 2, 3, 4, 5)");
/// });
/// # }
/// ```
///
/// # Panics
///
/// This function will panic if `element`'s [`ExactSizeIterator`] implementation is incorrect.
/// All standard library structures implement this trait correctly, if they do, so calling this
/// function using [`Vec`]`<T>` or `&[T]` will always succeed.
#[track_caller]
pub fn new<T, U>(py: Python, elements: impl IntoIterator<Item = T, IntoIter = U>) -> &PyTuple
where
T: ToPyObject,
U: ExactSizeIterator<Item = T>,
{
let elements_iter = elements.into_iter();
let len = elements_iter.len();
unsafe {
let ptr = ffi::PyTuple_New(len as Py_ssize_t);
for (i, e) in elements_iter.enumerate() {
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
ffi::PyTuple_SET_ITEM(ptr, i as Py_ssize_t, e.to_object(py).into_ptr());
#[cfg(any(Py_LIMITED_API, PyPy))]
ffi::PyTuple_SetItem(ptr, i as Py_ssize_t, e.to_object(py).into_ptr());
}
py.from_owned_ptr(ptr)
}
let elements = elements.into_iter();
let ptr = new_from_iter(py, elements, |e| e.to_object(py));
unsafe { py.from_owned_ptr(ptr) }
}
/// Constructs an empty tuple (on the Python side, a singleton object).
@ -247,8 +304,9 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
fn to_object(&self, py: Python) -> PyObject {
unsafe {
let ptr = ffi::PyTuple_New($length);
let ret = PyObject::from_owned_ptr(py, ptr);
$(ffi::PyTuple_SetItem(ptr, $n, self.$n.to_object(py).into_ptr());)+
PyObject::from_owned_ptr(py, ptr)
ret
}
}
}
@ -256,8 +314,9 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
fn into_py(self, py: Python) -> PyObject {
unsafe {
let ptr = ffi::PyTuple_New($length);
let ret = PyObject::from_owned_ptr(py, ptr);
$(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_py(py).into_ptr());)+
PyObject::from_owned_ptr(py, ptr)
ret
}
}
}
@ -266,8 +325,9 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
fn into_py(self, py: Python) -> Py<PyTuple> {
unsafe {
let ptr = ffi::PyTuple_New($length);
let ret = Py::from_owned_ptr(py, ptr);
$(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_py(py).into_ptr());)+
Py::from_owned_ptr(py, ptr)
ret
}
}
}
@ -396,7 +456,7 @@ tuple_conversion!(
#[cfg(test)]
mod tests {
use crate::types::{PyAny, PyTuple};
use crate::{PyTryFrom, Python, ToPyObject};
use crate::{IntoPy, Py, PyTryFrom, Python, ToPyObject};
use std::collections::HashSet;
#[test]
@ -696,4 +756,172 @@ mod tests {
assert!(tuple.index(42i32).is_err());
});
}
use std::ops::Range;
struct FaultyIter(Range<usize>, usize);
impl Iterator for FaultyIter {
type Item = usize;
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}
}
impl ExactSizeIterator for FaultyIter {
fn len(&self) -> usize {
self.1
}
}
#[test]
#[should_panic(
expected = "Attempted to create PyTuple but `elements` was larger than reported by its `ExactSizeIterator` implementation."
)]
fn too_long_iterator() {
Python::with_gil(|py| {
let iter = FaultyIter(0..usize::MAX, 73);
let _tuple = PyTuple::new(py, iter);
})
}
#[test]
#[should_panic(
expected = "Attempted to create PyTuple but `elements` was smaller than reported by its `ExactSizeIterator` implementation."
)]
fn too_short_iterator() {
Python::with_gil(|py| {
let iter = FaultyIter(0..35, 73);
let _tuple = PyTuple::new(py, iter);
})
}
#[test]
#[should_panic(
expected = "out of range integral type conversion attempted on `elements.len()`"
)]
fn overflowing_size() {
Python::with_gil(|py| {
let iter = FaultyIter(0..0, usize::MAX);
let _tuple = PyTuple::new(py, iter);
})
}
#[test]
fn bad_clone_mem_leaks() {
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0);
#[crate::pyclass]
#[pyo3(crate = "crate")]
struct Bad(usize);
impl Clone for Bad {
fn clone(&self) -> Self {
if self.0 == 42 {
panic!()
};
NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst);
Bad(self.0)
}
}
impl Drop for Bad {
fn drop(&mut self) {
NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst);
}
}
impl ToPyObject for Bad {
fn to_object(&self, py: Python) -> Py<PyAny> {
self.to_owned().into_py(py)
}
}
struct FaultyIter(Range<usize>, usize);
impl Iterator for FaultyIter {
type Item = Bad;
fn next(&mut self) -> Option<Self::Item> {
self.0.next().map(|i| {
NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst);
Bad(i)
})
}
}
impl ExactSizeIterator for FaultyIter {
fn len(&self) -> usize {
self.1
}
}
Python::with_gil(|py| {
let _ = std::panic::catch_unwind(|| {
let iter = FaultyIter(0..50, 50);
let _tuple = PyTuple::new(py, iter);
});
});
assert_eq!(
NEEDS_DESTRUCTING_COUNT.load(SeqCst),
0,
"Some destructors did not run"
);
}
#[test]
fn bad_clone_mem_leaks_2() {
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0);
#[crate::pyclass]
#[pyo3(crate = "crate")]
struct Bad(usize);
impl Clone for Bad {
fn clone(&self) -> Self {
if self.0 == 3 {
// This panic should not lead to a memory leak
panic!()
};
NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst);
Bad(self.0)
}
}
impl Drop for Bad {
fn drop(&mut self) {
NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst);
}
}
impl ToPyObject for Bad {
fn to_object(&self, py: Python) -> Py<PyAny> {
self.to_owned().into_py(py)
}
}
let s = (Bad(1), Bad(2), Bad(3), Bad(4));
NEEDS_DESTRUCTING_COUNT.store(4, SeqCst);
Python::with_gil(|py| {
let _ = std::panic::catch_unwind(|| {
let _tuple: Py<PyAny> = s.to_object(py);
});
});
drop(s);
assert_eq!(
NEEDS_DESTRUCTING_COUNT.load(SeqCst),
0,
"Some destructors did not run"
);
}
}