Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit f68c28b

Browse files
committed
Auto merge of #129845 - scottmcm:redo-layout, r=Noratrieb
Take more advantage of the `isize::MAX` limit in `Layout` Things like `padding_needed_for` are current implemented being super careful to handle things like `Layout::size` potentially being `usize::MAX`. But now that #95295 has happened, that's no longer a concern. It's possible to add two `Layout::size`s together without risking overflow now. So take advantage of that to remove a bunch of checked math that's not actually needed. For example, the round-up-and-add-next-size in `extend` doesn't need any overflow checks at all, just the final check for compatibility with the alignment. (And while I was doing that I made it all unstably const, because there's nothing in `Layout` that's fundamentally runtime-only.)
2 parents f6bcd09 + 18ca8bf commit f68c28b

File tree

2 files changed

+110
-56
lines changed

2 files changed

+110
-56
lines changed

‎library/core/src/alloc/layout.rs

Lines changed: 105 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
// Your performance intuition is useless. Run perf.
66

77
use crate::error::Error;
8+
use crate::intrinsics::{unchecked_add, unchecked_mul, unchecked_sub};
9+
use crate::mem::SizedTypeProperties;
810
use crate::ptr::{Alignment, NonNull};
9-
use crate::{assert_unsafe_precondition, cmp,fmt, mem};
11+
use crate::{assert_unsafe_precondition, fmt, mem};
1012

1113
// While this function is used in one place and its implementation
1214
// could be inlined, the previous attempts to do so made rustc
@@ -98,7 +100,10 @@ impl Layout {
98100
//
99101
// Above implies that checking for summation overflow is both
100102
// necessary and sufficient.
101-
isize::MAX as usize - (align.as_usize() - 1)
103+
104+
// SAFETY: the maximum possible alignment is `isize::MAX + 1`,
105+
// so the subtraction cannot overflow.
106+
unsafe { unchecked_sub(isize::MAX as usize + 1, align.as_usize()) }
102107
}
103108

104109
/// Internal helper constructor to skip revalidating alignment validity.
@@ -252,9 +257,14 @@ impl Layout {
252257
/// Returns an error if the combination of `self.size()` and the given
253258
/// `align` violates the conditions listed in [`Layout::from_size_align`].
254259
#[stable(feature = "alloc_layout_manipulation", since = "1.44.0")]
260+
#[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
255261
#[inline]
256-
pub fn align_to(&self, align: usize) -> Result<Self, LayoutError> {
257-
Layout::from_size_align(self.size(), cmp::max(self.align(), align))
262+
pub const fn align_to(&self, align: usize) -> Result<Self, LayoutError> {
263+
if let Some(align) = Alignment::new(align) {
264+
Layout::from_size_alignment(self.size, Alignment::max(self.align, align))
265+
} else {
266+
Err(LayoutError)
267+
}
258268
}
259269

260270
/// Returns the amount of padding we must insert after `self`
@@ -279,29 +289,42 @@ impl Layout {
279289
without modifying the `Layout`"]
280290
#[inline]
281291
pub const fn padding_needed_for(&self, align: usize) -> usize {
282-
let len = self.size();
292+
// FIXME: Can we just change the type on this to `Alignment`?
293+
let Some(align) = Alignment::new(align) else { return usize::MAX };
294+
let len_rounded_up = self.size_rounded_up_to_custom_align(align);
295+
// SAFETY: Cannot overflow because the rounded-up value is never less
296+
unsafe { unchecked_sub(len_rounded_up, self.size) }
297+
}
283298

299+
/// Returns the smallest multiple of `align` greater than or equal to `self.size()`.
300+
///
301+
/// This can return at most `Alignment::MAX` (aka `isize::MAX + 1`)
302+
/// because the original size is at most `isize::MAX`.
303+
#[inline]
304+
const fn size_rounded_up_to_custom_align(&self, align: Alignment) -> usize {
305+
// SAFETY:
284306
// Rounded up value is:
285-
// len_rounded_up = (len + align - 1) & !(align - 1);
286-
// and then we return the padding difference: `len_rounded_up - len`.
307+
// size_rounded_up = (size + align - 1) & !(align - 1);
287308
//
288-
// We use modular arithmetic throughout:
309+
// The arithmetic we do here can never overflow:
289310
//
290311
// 1. align is guaranteed to be > 0, so align - 1 is always
291312
// valid.
292313
//
293-
// 2. `len + align - 1` can overflow by at most `align - 1`,
294-
// so the &-mask with `!(align - 1)` will ensure that in the
295-
// case of overflow, `len_rounded_up` will itself be 0.
296-
// Thus the returned padding, when added to `len`, yields 0,
297-
// which trivially satisfies the alignment `align`.
314+
// 2. size is at most `isize::MAX`, so adding `align - 1` (which is at
315+
// most `isize::MAX`) can never overflow a `usize`.
298316
//
299-
// (Of course, attempts to allocate blocks of memory whose
300-
// size and padding overflow in the above manner should cause
301-
// the allocator to yield an error anyway.)
302-
303-
let len_rounded_up = len.wrapping_add(align).wrapping_sub(1) & !align.wrapping_sub(1);
304-
len_rounded_up.wrapping_sub(len)
317+
// 3. masking by the alignment can remove at most `align - 1`,
318+
// which is what we just added, thus the value we return is never
319+
// less than the original `size`.
320+
//
321+
// (Size 0 Align MAX is already aligned, so stays the same, but things like
322+
// Size 1 Align MAX or Size isize::MAX Align 2 round up to `isize::MAX + 1`.)
323+
unsafe {
324+
let align_m1 = unchecked_sub(align.as_usize(), 1);
325+
let size_rounded_up = unchecked_add(self.size, align_m1) & !align_m1;
326+
size_rounded_up
327+
}
305328
}
306329

307330
/// Creates a layout by rounding the size of this layout up to a multiple
@@ -315,12 +338,11 @@ impl Layout {
315338
without modifying the original"]
316339
#[inline]
317340
pub const fn pad_to_align(&self) -> Layout {
318-
let pad = self.padding_needed_for(self.align());
319341
// This cannot overflow. Quoting from the invariant of Layout:
320342
// > `size`, when rounded up to the nearest multiple of `align`,
321343
// > must not overflow isize (i.e., the rounded value must be
322344
// > less than or equal to `isize::MAX`)
323-
let new_size = self.size() + pad;
345+
let new_size = self.size_rounded_up_to_custom_align(self.align);
324346

325347
// SAFETY: padded size is guaranteed to not exceed `isize::MAX`.
326348
unsafe { Layout::from_size_align_unchecked(new_size, self.align()) }
@@ -333,20 +355,36 @@ impl Layout {
333355
/// layout of the array and `offs` is the distance between the start
334356
/// of each element in the array.
335357
///
358+
/// (That distance between elements is sometimes known as "stride".)
359+
///
336360
/// On arithmetic overflow, returns `LayoutError`.
361+
///
362+
/// # Examples
363+
///
364+
/// ```
365+
/// #![feature(alloc_layout_extra)]
366+
/// use std::alloc::Layout;
367+
///
368+
/// // All rust types have a size that's a multiple of their alignment.
369+
/// let normal = Layout::from_size_align(12, 4).unwrap();
370+
/// let repeated = normal.repeat(3).unwrap();
371+
/// assert_eq!(repeated, (Layout::from_size_align(36, 4).unwrap(), 12));
372+
///
373+
/// // But you can manually make layouts which don't meet that rule.
374+
/// let padding_needed = Layout::from_size_align(6, 4).unwrap();
375+
/// let repeated = padding_needed.repeat(3).unwrap();
376+
/// assert_eq!(repeated, (Layout::from_size_align(24, 4).unwrap(), 8));
377+
/// ```
337378
#[unstable(feature = "alloc_layout_extra", issue = "55724")]
379+
#[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
338380
#[inline]
339-
pub fn repeat(&self, n: usize) -> Result<(Self, usize), LayoutError> {
340-
// This cannot overflow. Quoting from the invariant of Layout:
341-
// > `size`, when rounded up to the nearest multiple of `align`,
342-
// > must not overflow isize (i.e., the rounded value must be
343-
// > less than or equal to `isize::MAX`)
344-
let padded_size = self.size() + self.padding_needed_for(self.align());
345-
let alloc_size = padded_size.checked_mul(n).ok_or(LayoutError)?;
346-
347-
// The safe constructor is called here to enforce the isize size limit.
348-
let layout = Layout::from_size_alignment(alloc_size, self.align)?;
349-
Ok((layout, padded_size))
381+
pub const fn repeat(&self, n: usize) -> Result<(Self, usize), LayoutError> {
382+
let padded = self.pad_to_align();
383+
if let Ok(repeated) = padded.repeat_packed(n) {
384+
Ok((repeated, padded.size()))
385+
} else {
386+
Err(LayoutError)
387+
}
350388
}
351389

352390
/// Creates a layout describing the record for `self` followed by
@@ -395,17 +433,23 @@ impl Layout {
395433
/// # assert_eq!(repr_c(&[u64, u32, u16, u32]), Ok((s, vec![0, 8, 12, 16])));
396434
/// ```
397435
#[stable(feature = "alloc_layout_manipulation", since = "1.44.0")]
436+
#[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
398437
#[inline]
399-
pub fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {
400-
let new_align = cmp::max(self.align, next.align);
401-
let pad = self.padding_needed_for(next.align());
402-
403-
let offset = self.size().checked_add(pad).ok_or(LayoutError)?;
404-
let new_size = offset.checked_add(next.size()).ok_or(LayoutError)?;
405-
406-
// The safe constructor is called here to enforce the isize size limit.
407-
let layout = Layout::from_size_alignment(new_size, new_align)?;
408-
Ok((layout, offset))
438+
pub const fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {
439+
let new_align = Alignment::max(self.align, next.align);
440+
let offset = self.size_rounded_up_to_custom_align(next.align);
441+
442+
// SAFETY: `offset` is at most `isize::MAX + 1` (such as from aligning
443+
// to `Alignment::MAX`) and `next.size` is at most `isize::MAX` (from the
444+
// `Layout` type invariant). Thus the largest possible `new_size` is
445+
// `isize::MAX + 1 + isize::MAX`, which is `usize::MAX`, and cannot overflow.
446+
let new_size = unsafe { unchecked_add(offset, next.size) };
447+
448+
if let Ok(layout) = Layout::from_size_alignment(new_size, new_align) {
449+
Ok((layout, offset))
450+
} else {
451+
Err(LayoutError)
452+
}
409453
}
410454

411455
/// Creates a layout describing the record for `n` instances of
@@ -421,11 +465,15 @@ impl Layout {
421465
///
422466
/// On arithmetic overflow, returns `LayoutError`.
423467
#[unstable(feature = "alloc_layout_extra", issue = "55724")]
468+
#[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
424469
#[inline]
425-
pub fn repeat_packed(&self, n: usize) -> Result<Self, LayoutError> {
426-
let size = self.size().checked_mul(n).ok_or(LayoutError)?;
427-
// The safe constructor is called here to enforce the isize size limit.
428-
Layout::from_size_alignment(size, self.align)
470+
pub const fn repeat_packed(&self, n: usize) -> Result<Self, LayoutError> {
471+
if let Some(size) = self.size.checked_mul(n) {
472+
// The safe constructor is called here to enforce the isize size limit.
473+
Layout::from_size_alignment(size, self.align)
474+
} else {
475+
Err(LayoutError)
476+
}
429477
}
430478

431479
/// Creates a layout describing the record for `self` followed by
@@ -435,10 +483,13 @@ impl Layout {
435483
///
436484
/// On arithmetic overflow, returns `LayoutError`.
437485
#[unstable(feature = "alloc_layout_extra", issue = "55724")]
486+
#[rustc_const_unstable(feature = "const_alloc_layout", issue = "67521")]
438487
#[inline]
439-
pub fn extend_packed(&self, next: Self) -> Result<Self, LayoutError> {
440-
let new_size = self.size().checked_add(next.size()).ok_or(LayoutError)?;
441-
// The safe constructor is called here to enforce the isize size limit.
488+
pub const fn extend_packed(&self, next: Self) -> Result<Self, LayoutError> {
489+
// SAFETY: each `size` is at most `isize::MAX == usize::MAX/2`, so the
490+
// sum is at most `usize::MAX/2*2 == usize::MAX - 1`, and cannot overflow.
491+
let new_size = unsafe { unchecked_add(self.size, next.size) };
492+
// The safe constructor enforces that the new size isn't too big for the alignment
442493
Layout::from_size_alignment(new_size, self.align)
443494
}
444495

@@ -451,14 +502,12 @@ impl Layout {
451502
#[inline]
452503
pub const fn array<T>(n: usize) -> Result<Self, LayoutError> {
453504
// Reduce the amount of code we need to monomorphize per `T`.
454-
return inner(mem::size_of::<T>(),Alignment::of::<T>(), n);
505+
return inner(T::LAYOUT, n);
455506

456507
#[inline]
457-
const fn inner(
458-
element_size: usize,
459-
align: Alignment,
460-
n: usize,
461-
) -> Result<Layout, LayoutError> {
508+
const fn inner(element_layout: Layout, n: usize) -> Result<Layout, LayoutError> {
509+
let Layout { size: element_size, align } = element_layout;
510+
462511
// We need to check two things about the size:
463512
// - That the total size won't overflow a `usize`, and
464513
// - That the total size still fits in an `isize`.
@@ -473,7 +522,7 @@ impl Layout {
473522
// This is a useless hint inside this function, but after inlining this helps
474523
// deduplicate checks for whether the overall capacity is zero (e.g., in RawVec's
475524
// allocation path) before/after this multiplication.
476-
let array_size = unsafe { element_size.unchecked_mul(n) };
525+
let array_size = unsafe { unchecked_mul(element_size,n) };
477526

478527
// SAFETY: We just checked above that the `array_size` will not
479528
// exceed `isize::MAX` even when rounded up to the alignment.

‎library/core/src/ptr/alignment.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ impl Alignment {
154154
// SAFETY: The alignment is always nonzero, and therefore decrementing won't overflow.
155155
!(unsafe { self.as_usize().unchecked_sub(1) })
156156
}
157+
158+
// Remove me once `Ord::max` is usable in const
159+
pub(crate) const fn max(a: Self, b: Self) -> Self {
160+
if a.as_usize() > b.as_usize() { a } else { b }
161+
}
157162
}
158163

159164
#[unstable(feature = "ptr_alignment_type", issue = "102070")]

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /