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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 25 commits into from
Closed

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

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
no more direct hole punch to alloc bytes
  • Loading branch information
Ellen Arteca committed Aug 4, 2022
commit e0e8e00a41824bd5b7c5d2a29adfd04743418e73
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/allocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::ty;
pub struct Allocation<Prov = AllocId, Extra = ()> {
/// The actual bytes of the allocation.
/// Note that the bytes of a pointer represent the offset of the pointer.
pub bytes: Box<[u8]>,
bytes: Box<[u8]>,
/// Maps from byte addresses to extra data for each pointer.
/// Only the first byte of a pointer is inserted into the map; i.e.,
/// every entry in this map applies to `pointer_size` consecutive bytes starting
Expand Down Expand Up @@ -352,6 +352,11 @@ impl<Prov, Extra> Allocation<Prov, Extra> {

/// Byte accessors.
impl<Prov: Provenance, Extra> Allocation<Prov, Extra> {
/// Get the pointer of the [u8] of bytes.
pub fn get_bytes_addr(&self) -> Size {
Copy link
Member

Choose a reason for hiding this comment

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

This is deliberately the host address, right? So it should have return type usize, not Size.

Also, to actually allow C code to use this address, we need to be a bit more careful with aliasing here. The pointer you are exposing here will be invalidated the next time someone mutates this memory, causing UB in Miri itself due to violating the Rust aliasing rules.

Under my proposal where bytes: Bytes, I think the AllocationBytes trait should have a method expose_base_addr with return type usize, and this function here simply calls expose_base_addr. (It should probably also be called expose_base_addr.)

Copy link
Author

Choose a reason for hiding this comment

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

Good point -- thanks for the suggestion, that makes sense -- AllocationBytes trait is a clean solution.

Size::from_bytes(self.bytes.as_ptr() as u64)
}

/// This is the entirely abstraction-violating way to just grab the raw bytes without
/// caring about relocations. It just deduplicates some code between `read_scalar`
/// and `get_bytes_internal`.
Expand Down