From 65da4f362e5d6f4fab38c0e66579abf80d9b4b5d Mon Sep 17 00:00:00 2001 From: Safe4U Date: Tue, 9 Jul 2024 16:33:08 +0800 Subject: [PATCH] Fix unsound problem in `Row::row_unchecked` (#5) Hi, thanks for your folk of arrow and best wishes first. We have found a simple unsound problem in `::compute::sort::row::Rows`. The function [`Rows::row_unchecked`](https://github.com/rerun-io/re_arrow2/blob/33a32000001df800e4840d92c33b03e7007311e1/src/compute/sort/row/mod.rs#L272) is wrongly marked as safe, which would confuse the boundary between safe and unsafe Rust. Since the repository of arrow2 have been archived, we cannot create any issue report. So we directly open this PR to mark the function `row_unchecked` as 'unsafe' and add a `debug_assert!` macro for safety consideration. --- src/compute/sort/row/mod.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/compute/sort/row/mod.rs b/src/compute/sort/row/mod.rs index 2388a6c8680..7d27e5a1f58 100644 --- a/src/compute/sort/row/mod.rs +++ b/src/compute/sort/row/mod.rs @@ -260,6 +260,10 @@ pub struct Rows { impl Rows { /// Get a reference to a certain row. + /// + /// # Panics + /// + /// Panics if `row` is out of bounds pub fn row(&self, row: usize) -> Row<'_> { let end = self.offsets[row + 1]; let start = self.offsets[row]; @@ -269,7 +273,12 @@ impl Rows { } /// Get a reference to a certain row but not check the bounds. - pub fn row_unchecked(&self, row: usize) -> Row<'_> { + /// + /// # Safety + /// + /// The caller must ensure that `row` is within bounds + pub unsafe fn row_unchecked(&self, row: usize) -> Row<'_> { + debug_assert!(row < self.len(), "out-of-bounds access"); let data = unsafe { let end = *self.offsets.get_unchecked(row + 1); let start = *self.offsets.get_unchecked(row); @@ -319,7 +328,7 @@ impl<'a> Iterator for RowsIter<'a> { #[inline] fn next(&mut self) -> Option { if self.start < self.end { - let row = self.rows.row_unchecked(self.start); + let row = unsafe { self.rows.row_unchecked(self.start) }; self.start += 1; Some(row) } else {