From 123df596c77489bd6a520b453365bc59cbd35db2 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Mon, 17 Feb 2020 18:55:41 +0100 Subject: [PATCH] Revert "Remove `checked_add` in `Layout::repeat`" This fixes a a segfault in safe code, a stable regression. Reported in \#69225. This reverts commit a983e0590a43ed8b0f60417828efd4e79b51f494. Also adds a test for the expected behaviour. --- src/libcore/alloc.rs | 12 ++++--- ...issue-69225-layout-repeated-checked-add.rs | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/issues/issue-69225-layout-repeated-checked-add.rs diff --git a/src/libcore/alloc.rs b/src/libcore/alloc.rs index 09f743fb81e4c..2050f64512a23 100644 --- a/src/libcore/alloc.rs +++ b/src/libcore/alloc.rs @@ -241,11 +241,13 @@ impl Layout { #[unstable(feature = "alloc_layout_extra", issue = "55724")] #[inline] pub fn repeat(&self, n: usize) -> Result<(Self, usize), LayoutErr> { - // This cannot overflow. Quoting from the invariant of Layout: - // > `size`, when rounded up to the nearest multiple of `align`, - // > must not overflow (i.e., the rounded value must be less than - // > `usize::MAX`) - let padded_size = self.size() + self.padding_needed_for(self.align()); + // Warning, removing the checked_add here led to segfaults in #67174. Further + // analysis in #69225 seems to indicate that this is an LTO-related + // miscompilation, so #67174 might be able to be reapplied in the future. + let padded_size = self + .size() + .checked_add(self.padding_needed_for(self.align())) + .ok_or(LayoutErr { private: () })?; let alloc_size = padded_size.checked_mul(n).ok_or(LayoutErr { private: () })?; unsafe { diff --git a/src/test/ui/issues/issue-69225-layout-repeated-checked-add.rs b/src/test/ui/issues/issue-69225-layout-repeated-checked-add.rs new file mode 100644 index 0000000000000..7f43e4d1a51f8 --- /dev/null +++ b/src/test/ui/issues/issue-69225-layout-repeated-checked-add.rs @@ -0,0 +1,31 @@ +// Ensure we appropriately error instead of overflowing a calculation when creating a new Alloc +// Layout + +// run-fail +// compile-flags: -C opt-level=3 +// error-pattern: index out of bounds: the len is 0 but the index is 16777216 +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support + +fn do_test(x: usize) { + let arr = vec![vec![0u8; 3]]; + + let mut z = Vec::new(); + for arr_ref in arr { + for y in 0..x { + for _ in 0..1 { + z.extend(std::iter::repeat(0).take(x)); + let a = y * x; + let b = (y + 1) * x - 1; + let slice = &arr_ref[a..b]; + eprintln!("{} {} {} {}", a, b, arr_ref.len(), slice.len()); + eprintln!("{:?}", slice[1 << 24]); + } + } + } +} + +fn main() { + do_test(1); + do_test(2); +}