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

Alignment of the bytes of Allocation to match align parameter #100467

Closed
wants to merge 25 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0fe75b8
Stop x.py from being mean
maurer Aug 2, 2022
f37fe37
DO NOT MERGE - scratchwork
maurer Aug 3, 2022
fde4235
point to my miri fork
Aug 3, 2022
e0e8e00
no more direct hole punch to alloc bytes
Aug 4, 2022
a7b7f19
no more holepunch to bytes
Aug 4, 2022
f7a991b
commented out alignment check for int-aligned addrs -- need to make t…
Aug 6, 2022
e2ed272
proper check for size of allocation being aligned
Aug 7, 2022
51269b8
using the align parameter to properly align allocations
Aug 8, 2022
bca203e
redoing allocation bytes realignment -- still testing with miri
Aug 12, 2022
0ddff36
allocation bytes alignment, and cleanup .gitmodules
Aug 12, 2022
a72a057
double free detected in tcache 2: could not compile core in stage 1 c…
Aug 12, 2022
04f29dc
using slice directly, no intermediate vec
Aug 12, 2022
2fd7606
removing miri submodule updates
Aug 15, 2022
ab1a61f
removing miri submodule updates
Aug 15, 2022
b87f5ef
removing miri submodule updates
Aug 15, 2022
cade1c1
going back to previous version of miri to match commit of rustc
Aug 15, 2022
c31d404
moving AllocBytes into a trait -- partially done
emarteca Sep 5, 2022
e993680
more moving allocbytes into trait
emarteca Sep 7, 2022
17ac36b
allocbytes moved into a trait, implemented for `Box<[u8]>` as default
emarteca Sep 7, 2022
c2e142b
merging in changes from upstream master
emarteca Sep 8, 2022
99f6708
cleanup
emarteca Sep 8, 2022
a12d111
cleanup
emarteca Sep 8, 2022
f75649b
propagating Allocation Bytes type
emarteca Sep 14, 2022
8db066f
adding deref and derefmut to allocbytes, removing now unnecessary met…
emarteca Nov 9, 2022
f075a12
nit
emarteca Nov 9, 2022
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
Prev Previous commit
Next Next commit
allocbytes moved into a trait, implemented for Box<[u8]> as default
  • Loading branch information
emarteca committed Sep 7, 2022
commit 17ac36b5b18df3d050bb4ebb6fd65f5a681346ac
101 changes: 16 additions & 85 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use std::hash::Hash;
use std::iter;
use std::ops::{Deref, Range};
use std::ptr;
use std::mem::MaybeUninit;

use rustc_ast::Mutability;
use rustc_data_structures::intern::Interned;
Expand All @@ -24,76 +23,33 @@ use super::{
};
use crate::ty;

/// Representation of a section of memory, starting at a particular
/// address and of a specified length.
/// This is how we represent bytes in an `Allocation` that can't be
/// owned, since they belong to a foreign process -- in particular, we
/// use this to store pointers to C memory passed back from C FFI calls
/// in Miri.
// TODO! ellen move this into Miri
// #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
// #[derive(TyEncodable, TyDecodable)]
// pub struct AddrAllocBytes {
// /// Address of the beginning of the bytes.
// pub addr: u64,
// /// Size of the type of the data being stored in these bytes.
// pub type_size: usize,
// /// Length of the bytes, in multiples of `type_size`;
// /// it's in a `RefCell` since it can change depending on how it's used
// /// in the program. UNSAFE
// pub len: std::cell::RefCell<usize>,
// }

// impl AddrAllocBytes {
// /// Length of the bytes.
// pub fn total_len(&self) -> usize {
// self.type_size * *self.len.borrow()
// }
// }

// // Satisfy the `Hash` and `HashStable` trait requirements; can't be automatically derived.
// impl hash::Hash for AddrAllocBytes {
// fn hash<H: hash::Hasher>(&self, state: &mut H) {
// self.addr.hash(state);
// self.type_size.hash(state);
// }
// }
// impl<CTX> HashStable<CTX> for AddrAllocBytes {
// fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) {
// self.addr.hash_stable(hcx, hasher);
// self.type_size.hash_stable(hcx, hasher);
// }
// }

/// Types that can be used to represent the `bytes field of an `Allocation`.
// #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
// #[derive(TyEncodable, TyDecodable)]
// #[derive(HashStable)]
// pub enum AllocBytes {
// /// Owned, boxed slice of [u8].
// Boxed(Box<[u8]>),
// /// Address, size of the type stored, and length of the allocation.
// /// This is used for representing pointers to bytes that belong to a
// /// foreign process (such as pointers into C memory, passed back to Rust
// /// through an FFI call).
// Addr(AddrAllocBytes),
// }

/// Functionality required for the bytes of an `Allocation`.
pub trait AllocBytes: Clone + core::fmt::Debug + Eq + PartialEq + PartialOrd + Ord + core::hash::Hash {
/// The length of the bytes.
fn get_len(&self) -> usize;
fn get_addr(&self) -> u64;
/// The address of the bytes.
fn expose_addr(&self) -> u64;
Comment on lines +37 to +38
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to make this return a raw pointer, and leave exposing and things like that to the caller.

And in fact... why is this even needed? One can use the deref to get such a pointer, right?

/// Get a slice of the bytes corresponding to a specified range.
fn get_slice_from_range(&self, range: Range<usize>) -> &[u8];
/// Mutable slice of the bytes corresponding to a specified range.
fn get_slice_from_range_mut<'a>(&'a mut self, range: Range<usize>) -> &'a mut [u8];
/// Add to the pointer of the head of the bytes, and return a mutable pointer to this location.
fn add_ptr(&mut self, to_add: usize) -> *mut u8;
fn write_maybe_uninit_slice(boxed: &mut Box<[MaybeUninit<u8>]>, to_write: &Self);
/// Hash the head and tail of the bytes.
/// This is required to statisfy the `Hash` trait.
fn hash_head_tail<H: hash::Hasher>(&self, _byte_count: usize, _state: &mut H, _max_bytes_to_hash: usize) {}
/// Adjust the bytes to the specified alignment -- by default, this is a no-op.
fn adjust_to_align(self, _align: Align) -> Self {
self
}
Copy link
Member

Choose a reason for hiding this comment

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

What is this about? Alignment should be set at allocation time.

/// Create an `AllocBytes` from a slice of `u8`.
fn from_bytes<'a>(slice: impl Into<Cow<'a, [u8]>>, _align: Align) -> Self;
/// Create an uninitialized `AllocBytes` of the specified size and alignment;
/// call the callback error handler if there is an error in allocating the memory.
fn uninit<'tcx, F: Fn() -> InterpError<'tcx>>(size: Size, _align: Align, handle_alloc_fail: F) -> Result<Self, InterpError<'tcx>>;
}

// Default `bytes` for `Allocation` is a `Box<[u8]>`.
impl AllocBytes for Box<[u8]> {

fn uninit<'tcx, F: Fn() -> InterpError<'tcx>>(size: Size, _align: Align, handle_alloc_fail: F) -> Result<Self, InterpError<'tcx>> {
Expand All @@ -115,7 +71,7 @@ impl AllocBytes for Box<[u8]> {
}

/// The real address of the bytes.
fn get_addr(&self) -> u64 {
fn expose_addr(&self) -> u64 {
self.as_ptr() as u64
}

Expand All @@ -134,12 +90,6 @@ impl AllocBytes for Box<[u8]> {
self.as_mut_ptr().wrapping_add(to_add)
}

/// Write an `AllocBytes` to a boxed slice of `MaybeUninit` -- this serves to initialize
/// the elements in `boxed`, for the length of the `AllocBytes` passed in.
fn write_maybe_uninit_slice(boxed: &mut Box<[MaybeUninit<u8>]>, to_write: &Self) {
MaybeUninit::write_slice(boxed, &to_write);
}

fn hash_head_tail<H: hash::Hasher>(&self, _byte_count: usize, _state: &mut H, _max_bytes_to_hash: usize) {
self[.._max_bytes_to_hash].hash(_state);
self[_byte_count - _max_bytes_to_hash..].hash(_state);
Expand Down Expand Up @@ -345,25 +295,6 @@ impl<Prov, Bytes: AllocBytes> Allocation<Prov, (), Bytes> {
}
}

// pub fn from_raw_addr(
// addr: u64,
// type_size: usize,
// len: usize,
// align: Align,
// mutability: Mutability,
// ) -> Self {
// let addr_alloc_bytes = AddrAllocBytes { addr, type_size, len: std::cell::RefCell::new(len)};
// let size = Size::from_bytes(addr_alloc_bytes.total_len());
// Self {
// bytes: AllocBytes::Addr(addr_alloc_bytes),
// relocations: Relocations::new(),
// init_mask: InitMask::new(size, true),
// align,
// mutability,
// extra: (),
// }
// }

pub fn from_bytes_byte_aligned_immutable<'a>(slice: impl Into<Cow<'a, [u8]>>) -> Self {
Allocation::from_bytes(slice, Align::ONE, Mutability::Not)
}
Expand Down Expand Up @@ -473,7 +404,7 @@ impl<Prov, Extra> Allocation<Prov, Extra> {
impl<Prov: Provenance, Extra> Allocation<Prov, Extra> {
/// Get the pointer of the [u8] of bytes.
pub fn expose_base_addr(&self) -> usize {
self.bytes.get_addr().try_into().unwrap()
self.bytes.expose_addr().try_into().unwrap()
}

/// This is the entirely abstraction-violating way to just grab the raw bytes without
Expand Down