Skip to content

Commit ed3e39f

Browse files
committed
fix unsoundness in Step::forward_unchecked for signed integers
1 parent bfe762e commit ed3e39f

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

library/core/src/iter/range.rs

+29-2
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,26 @@ pub trait Step: Clone + PartialOrd + Sized {
184184
}
185185
}
186186

187-
// These are still macro-generated because the integer literals resolve to different types.
188-
macro_rules! step_identical_methods {
187+
188+
// Separate impls for signed ranges because the distance within a signed range can be larger
189+
// than the signed::MAX value. THerefore `as` casting to the signed type would be incorrect.
190+
macro_rules! step_signed_methods {
191+
($unsigned: ty) => {
192+
#[inline]
193+
unsafe fn forward_unchecked(start: Self, n: usize) -> Self {
194+
// SAFETY: the caller has to guarantee that `start + n` doesn't overflow.
195+
unsafe { start.checked_add_unsigned(n as $unsigned).unwrap_unchecked() }
196+
}
197+
198+
#[inline]
199+
unsafe fn backward_unchecked(start: Self, n: usize) -> Self {
200+
// SAFETY: the caller has to guarantee that `start - n` doesn't overflow.
201+
unsafe { start.checked_sub_unsigned(n as $unsigned).unwrap_unchecked() }
202+
}
203+
}
204+
}
205+
206+
macro_rules! step_unsigned_methods {
189207
() => {
190208
#[inline]
191209
unsafe fn forward_unchecked(start: Self, n: usize) -> Self {
@@ -198,7 +216,12 @@ macro_rules! step_identical_methods {
198216
// SAFETY: the caller has to guarantee that `start - n` doesn't overflow.
199217
unsafe { start.unchecked_sub(n as Self) }
200218
}
219+
}
220+
}
201221

222+
// These are still macro-generated because the integer literals resolve to different types.
223+
macro_rules! step_identical_methods {
224+
() => {
202225
#[inline]
203226
#[allow(arithmetic_overflow)]
204227
#[rustc_inherit_overflow_checks]
@@ -239,6 +262,7 @@ macro_rules! step_integer_impls {
239262
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
240263
impl Step for $u_narrower {
241264
step_identical_methods!();
265+
step_unsigned_methods!();
242266

243267
#[inline]
244268
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
@@ -271,6 +295,7 @@ macro_rules! step_integer_impls {
271295
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
272296
impl Step for $i_narrower {
273297
step_identical_methods!();
298+
step_signed_methods!($u_narrower);
274299

275300
#[inline]
276301
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
@@ -335,6 +360,7 @@ macro_rules! step_integer_impls {
335360
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
336361
impl Step for $u_wider {
337362
step_identical_methods!();
363+
step_unsigned_methods!();
338364

339365
#[inline]
340366
fn steps_between(start: &Self, end: &Self) -> Option<usize> {
@@ -360,6 +386,7 @@ macro_rules! step_integer_impls {
360386
#[unstable(feature = "step_trait", reason = "recently redesigned", issue = "42168")]
361387
impl Step for $i_wider {
362388
step_identical_methods!();
389+
step_signed_methods!($u_wider);
363390

364391
#[inline]
365392
fn steps_between(start: &Self, end: &Self) -> Option<usize> {

library/core/tests/iter/range.rs

+5
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,11 @@ fn test_range_advance_by() {
325325
assert_eq!(Ok(()), r.advance_back_by(usize::MAX));
326326

327327
assert_eq!((r.start, r.end), (0u128 + usize::MAX as u128, u128::MAX - usize::MAX as u128));
328+
329+
// issue 122420, Step::forward_unchecked was unsound for signed integers
330+
let mut r = -128i8..127;
331+
assert_eq!(Ok(()), r.advance_by(200));
332+
assert_eq!(r.next(), Some(72));
328333
}
329334

330335
#[test]

0 commit comments

Comments
 (0)