Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic early when TrustedLen indicates a length > usize::MAX #83726

Merged
merged 1 commit into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
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
7 changes: 5 additions & 2 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1848,8 +1848,11 @@ impl<T, I: iter::TrustedLen<Item = T>> ToRcSlice<T> for I {
Rc::from_iter_exact(self, low)
}
} else {
// Fall back to normal implementation.
self.collect::<Vec<T>>().into()
// TrustedLen contract guarantees that `upper_bound == `None` implies an iterator
// length exceeding `usize::MAX`.
// The default implementation would collect into a vec which would panic.
// Thus we panic here immediately without invoking `Vec` code.
panic!("capacity overflow");
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2481,8 +2481,11 @@ impl<T, I: iter::TrustedLen<Item = T>> ToArcSlice<T> for I {
Arc::from_iter_exact(self, low)
}
} else {
// Fall back to normal implementation.
self.collect::<Vec<T>>().into()
// TrustedLen contract guarantees that `upper_bound == `None` implies an iterator
// length exceeding `usize::MAX`.
// The default implementation would collect into a vec which would panic.
// Thus we panic here immediately without invoking `Vec` code.
panic!("capacity overflow");
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ impl<T> Vec<T> {
///
/// [Capacity and reallocation]: #capacity-and-reallocation
///
/// # Panics
///
/// Panics if the new capacity exceeds `isize::MAX` bytes.
///
/// # Examples
///
/// ```
Expand Down Expand Up @@ -541,6 +545,10 @@ impl<T, A: Allocator> Vec<T, A> {
///
/// [Capacity and reallocation]: #capacity-and-reallocation
///
/// # Panics
///
/// Panics if the new capacity exceeds `isize::MAX` bytes.
///
/// # Examples
///
/// ```
Expand Down
7 changes: 6 additions & 1 deletion library/alloc/src/vec/spec_extend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ where
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the above code seemed to have the same condition written twice? See 10 lines above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One checks that high == low. the other checks that it's finite.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean

        if let Some(high_value) = high {
            debug_assert_eq!(
                low,
                high_value,
                "TrustedLen iterator's size hint is not exact: {:?}",
                (low, high)
            );
        }
        if let Some(additional) = high {
            self.reserve(additional);
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, probably just an historic artifact of code being moved around and redundant conditions not being merged. Someone just has to clean it up.

}
} else {
self.extend_desugared(iterator)
// Per TrustedLen contract a `None` upper bound means that the iterator length
// truly exceeds usize::MAX, which would eventually lead to a capacity overflow anyway.
// Since the other branch already panics eagerly (via `reserve()`) we do the same here.
// This avoids additional codegen for a fallback code path which would eventually
// panic anyway.
panic!("capacity overflow");
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions library/alloc/src/vec/spec_from_iter_nested.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ where
fn from_iter(iterator: I) -> Self {
let mut vector = match iterator.size_hint() {
(_, Some(upper)) => Vec::with_capacity(upper),
_ => Vec::new(),
// TrustedLen contract guarantees that `size_hint() == (_, None)` means that there
// are more than `usize::MAX` elements.
// Since the previous branch would eagerly panic if the capacity is too large
// (via `with_capacity`) we do the same here.
_ => panic!("capacity overflow"),
};
// must delegate to spec_extend() since extend() itself delegates
// to spec_from for empty Vecs
// reuse extend specialization for TrustedLen
vector.spec_extend(iterator);
vector
}
Expand Down