From ce338046c8b40e3284707d2ab725e9f076592959 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sun, 19 Jul 2020 13:45:51 +0200 Subject: [PATCH 1/2] Fix panic message when `RangeFrom` index is out of bounds Before, the `Range` method was called with `end = slice.len()`. Unfortunately, because `Range::index` first checks the order of the indices (start has to be smaller than end), an out of bounds index leads to `core::slice::slice_index_order_fail` being called. This prints the message 'slice index starts at 27 but ends at 10', which is worse than 'index 27 out of range for slice of length 10'. This is not only useful to normal users reading panic messages, but also for people inspecting assembly and being confused by `slice_index_order_fail` calls. --- src/libcore/slice/mod.rs | 10 ++++++++-- src/libcore/tests/slice.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 20b2c3d3c965a..52babaf9f72e6 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -3241,12 +3241,18 @@ unsafe impl SliceIndex<[T]> for ops::RangeFrom { #[inline] fn index(self, slice: &[T]) -> &[T] { - (self.start..slice.len()).index(slice) + if self.start > slice.len() { + slice_index_len_fail(self.start, slice.len()); + } + unsafe { &*self.get_unchecked(slice) } } #[inline] fn index_mut(self, slice: &mut [T]) -> &mut [T] { - (self.start..slice.len()).index_mut(slice) + if self.start > slice.len() { + slice_index_len_fail(self.start, slice.len()); + } + unsafe { &mut *self.get_unchecked_mut(slice) } } } diff --git a/src/libcore/tests/slice.rs b/src/libcore/tests/slice.rs index fba73be92be09..8e240832c13b8 100644 --- a/src/libcore/tests/slice.rs +++ b/src/libcore/tests/slice.rs @@ -1088,7 +1088,7 @@ mod slice_index { good: data[6..] == []; bad: data[7..]; - message: "but ends at"; // perhaps not ideal + message: "out of range"; } in mod rangeto_len { From 0d64b016398f9c0116330089f038b11e2a3c4e1d Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sun, 19 Jul 2020 16:12:50 +0200 Subject: [PATCH 2/2] Slightly improve panic messages when range indices are out of bounds --- src/libcore/slice/mod.rs | 19 +++++++++++++------ src/test/codegen/issue-69101-bounds-check.rs | 6 +++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 52babaf9f72e6..64d218c3650ac 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -2974,8 +2974,15 @@ where #[inline(never)] #[cold] #[track_caller] -fn slice_index_len_fail(index: usize, len: usize) -> ! { - panic!("index {} out of range for slice of length {}", index, len); +fn slice_start_index_len_fail(index: usize, len: usize) -> ! { + panic!("range start index {} out of range for slice of length {}", index, len); +} + +#[inline(never)] +#[cold] +#[track_caller] +fn slice_end_index_len_fail(index: usize, len: usize) -> ! { + panic!("range end index {} out of range for slice of length {}", index, len); } #[inline(never)] @@ -3160,7 +3167,7 @@ unsafe impl SliceIndex<[T]> for ops::Range { if self.start > self.end { slice_index_order_fail(self.start, self.end); } else if self.end > slice.len() { - slice_index_len_fail(self.end, slice.len()); + slice_end_index_len_fail(self.end, slice.len()); } unsafe { &*self.get_unchecked(slice) } } @@ -3170,7 +3177,7 @@ unsafe impl SliceIndex<[T]> for ops::Range { if self.start > self.end { slice_index_order_fail(self.start, self.end); } else if self.end > slice.len() { - slice_index_len_fail(self.end, slice.len()); + slice_end_index_len_fail(self.end, slice.len()); } unsafe { &mut *self.get_unchecked_mut(slice) } } @@ -3242,7 +3249,7 @@ unsafe impl SliceIndex<[T]> for ops::RangeFrom { #[inline] fn index(self, slice: &[T]) -> &[T] { if self.start > slice.len() { - slice_index_len_fail(self.start, slice.len()); + slice_start_index_len_fail(self.start, slice.len()); } unsafe { &*self.get_unchecked(slice) } } @@ -3250,7 +3257,7 @@ unsafe impl SliceIndex<[T]> for ops::RangeFrom { #[inline] fn index_mut(self, slice: &mut [T]) -> &mut [T] { if self.start > slice.len() { - slice_index_len_fail(self.start, slice.len()); + slice_start_index_len_fail(self.start, slice.len()); } unsafe { &mut *self.get_unchecked_mut(slice) } } diff --git a/src/test/codegen/issue-69101-bounds-check.rs b/src/test/codegen/issue-69101-bounds-check.rs index 8ade583b57127..a3aca3a2912a6 100644 --- a/src/test/codegen/issue-69101-bounds-check.rs +++ b/src/test/codegen/issue-69101-bounds-check.rs @@ -12,7 +12,7 @@ // CHECK-LABEL: @already_sliced_no_bounds_check #[no_mangle] pub fn already_sliced_no_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { - // CHECK: slice_index_len_fail + // CHECK: slice_end_index_len_fail // CHECK-NOT: panic_bounds_check let _ = (&a[..2048], &b[..2048], &mut c[..2048]); for i in 0..1024 { @@ -23,7 +23,7 @@ pub fn already_sliced_no_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { // CHECK-LABEL: @already_sliced_no_bounds_check_exact #[no_mangle] pub fn already_sliced_no_bounds_check_exact(a: &[u8], b: &[u8], c: &mut [u8]) { - // CHECK: slice_index_len_fail + // CHECK: slice_end_index_len_fail // CHECK-NOT: panic_bounds_check let _ = (&a[..1024], &b[..1024], &mut c[..1024]); for i in 0..1024 { @@ -35,7 +35,7 @@ pub fn already_sliced_no_bounds_check_exact(a: &[u8], b: &[u8], c: &mut [u8]) { // CHECK-LABEL: @already_sliced_bounds_check #[no_mangle] pub fn already_sliced_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { - // CHECK: slice_index_len_fail + // CHECK: slice_end_index_len_fail // CHECK: panic_bounds_check let _ = (&a[..1023], &b[..2048], &mut c[..2048]); for i in 0..1024 {