Skip to content

Commit

Permalink
Remove overalignment from align < 8 allocations
Browse files Browse the repository at this point in the history
This reorganizes the implementation of the System allocator to permit
adding various debuggig features. Currently, all that this implements is
a scheme that allocates a little extra space for low-alignment
allocations then returns a pointer into the actual allocation which is
offset so that it is not over-aligned.

This is a huge aid in discovering accidental reliance on over-alignment.
Allocators designed for C can be relied upon to produce over-aligned
pointers, so alignment-related bugs can be latent for a long time.

This implementation is also factored so accommodate other patches to the
default allocator which can help in detecting other sources of UB.
  • Loading branch information
saethlin committed Aug 12, 2022
1 parent f22819b commit d641a1d
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 16 deletions.
122 changes: 122 additions & 0 deletions library/std/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,128 @@ pub use alloc_crate::alloc::*;
#[derive(Debug, Default, Copy, Clone)]
pub struct System;

use crate::sys::alloc::System as Imp;

// When debug assertions are not enabled, `System` just forwards down to the particular platform
// implementation.
#[cfg(not(debug_assertions))]
#[stable(feature = "alloc_system_type", since = "1.28.0")]
unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
unsafe { Imp.alloc(layout) }
}

#[inline]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
unsafe { Imp.alloc_zeroed(layout) }
}

#[inline]
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
unsafe { Imp.dealloc(ptr, layout) }
}

#[inline]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
unsafe { Imp.realloc(ptr, layout, new_size) }
}
}

// Some system allocators (most notably any provided by calling malloc) will always return pointers
// with an alignment of 8. So for any allocation with an alignment less than 8, we increase the
// alignment to 8 and return a pointer which is offset into the allocation such that it is not
// over-aligned.
// We always bump up the size of an allocation by 8 when the alignment is less than 8.
#[cfg(debug_assertions)]
trait LayoutExt {
fn with_alignment_padding(self) -> Self;
unsafe fn add_alignment_padding(self, ptr: *mut u8) -> *mut u8;
unsafe fn remove_alignment_padding(self, ptr: *mut u8) -> *mut u8;
}
#[cfg(debug_assertions)]
impl LayoutExt for Layout {
fn with_alignment_padding(self) -> Self {
if self.align() < 8 {
Layout::from_size_align(self.size() + (8 - self.align()), 8).unwrap()
} else {
self
}
}

unsafe fn add_alignment_padding(self, ptr: *mut u8) -> *mut u8 {
if !ptr.is_null() && self.align() < 8 {
// SAFETY: This must be called on a pointer previously returned by a padded Layout,
// which will always have space to do this offset
unsafe { ptr.add(8 - self.align()) }
} else {
ptr
}
}

unsafe fn remove_alignment_padding(self, ptr: *mut u8) -> *mut u8 {
// We cannot just do the inverse of add_alignment_padding, because if a user deallocates
// with the wrong Layout, we would use that wrong Layout here to deduce the wrong offset to
// remove from the pointer. That would turn code that works fine because the underlying
// allocator ignores the Layout (but is technically UB) into code which does worse UB or
// halts the program with an unhelpful diagnostic from the underlying allocator.
// So we have two reasonable options. We could detect and clearly report the error
// ourselves, or since we know that all our alignment adjustments involve the low 3 bits,
// we could clear those and make this allocator transparent.
// At the moment we do the latter because it is unclear how to emit an error message from
// inside an allocator.
const ALIGNMENT_MASK: usize = usize::MAX << 3;
ptr.map_addr(|addr| addr & ALIGNMENT_MASK)
}
}

// When debug assertions are enabled, we wrap the platform allocator with extra logic to help
// expose bugs.
#[cfg(debug_assertions)]
#[stable(feature = "alloc_system_type", since = "1.28.0")]
unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
if layout.size() > isize::MAX as usize - 8 {
return ptr::null_mut();
}
unsafe {
let ptr = Imp.alloc(layout.with_alignment_padding());
layout.add_alignment_padding(ptr)
}
}

#[inline]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
unsafe {
let ptr = Imp.alloc_zeroed(layout.with_alignment_padding());
layout.add_alignment_padding(ptr)
}
}

#[inline]
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
unsafe {
let ptr = layout.remove_alignment_padding(ptr);
Imp.dealloc(ptr, layout.with_alignment_padding())
}
}

#[inline]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
if new_size > isize::MAX as usize - 8 {
return ptr::null_mut();
}
unsafe {
let ptr = layout.remove_alignment_padding(ptr);
let new_layout =
Layout::from_size_align(new_size, layout.align()).unwrap().with_alignment_padding();
let ptr = Imp.realloc(ptr, layout.with_alignment_padding(), new_layout.size());
layout.add_alignment_padding(ptr)
}
}
}

impl System {
#[inline]
fn alloc_impl(&self, layout: Layout, zeroed: bool) -> Result<NonNull<[u8]>, AllocError> {
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/common/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::cmp;
use crate::ptr;

Expand Down Expand Up @@ -36,7 +36,7 @@ pub const MIN_ALIGN: usize = 16;
pub const MIN_ALIGN: usize = 4;

pub unsafe fn realloc_fallback(
alloc: &System,
alloc: &impl GlobalAlloc,
ptr: *mut u8,
old_layout: Layout,
new_size: usize,
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/hermit/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::ptr;
use crate::sys::hermit::abi;

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/sgx/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::ptr;
use crate::sys::sgx::abi::mem as sgx_mem;
use core::sync::atomic::{AtomicBool, Ordering};
Expand Down Expand Up @@ -56,7 +56,9 @@ unsafe impl dlmalloc::Allocator for Sgx {
}
}

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/solid/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use crate::{
alloc::{GlobalAlloc, Layout, System},
alloc::{GlobalAlloc, Layout},
sys::common::alloc::{realloc_fallback, MIN_ALIGN},
};

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/unix/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::ptr;
use crate::sys::common::alloc::{realloc_fallback, MIN_ALIGN};

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/unsupported/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::ptr::null_mut;

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, _layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/wasm/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@
//! The crate itself provides a global allocator which on wasm has no
//! synchronization as there are no threads!
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};

static mut DLMALLOC: dlmalloc::Dlmalloc = dlmalloc::Dlmalloc::new();

#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/windows/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![deny(unsafe_op_in_unsafe_fn)]

use crate::alloc::{GlobalAlloc, Layout, System};
use crate::alloc::{GlobalAlloc, Layout};
use crate::ffi::c_void;
use crate::ptr;
use crate::sync::atomic::{AtomicPtr, Ordering};
Expand Down Expand Up @@ -187,7 +187,9 @@ unsafe fn allocate(layout: Layout, zeroed: bool) -> *mut u8 {
// the pointer will be aligned to the specified alignment and not point to the start of the allocated block.
// Instead there will be a header readable directly before the returned pointer, containing the actual
// location of the start of the block.
#[stable(feature = "alloc_system_type", since = "1.28.0")]
#[derive(Debug, Default, Copy, Clone)]
pub(crate) struct System;

unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
Expand Down

0 comments on commit d641a1d

Please sign in to comment.