Skip to content

Commit 7c01a57

Browse files
committed
Work around unaligned edges.
We load/store edges using ptr::{read,write}_unaligned on x86 and x86_64 to workaround unaligned edges in code roots.
1 parent 91e163a commit 7c01a57

File tree

5 files changed

+111
-18
lines changed

5 files changed

+111
-18
lines changed

mmtk/src/api.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ pub extern "C" fn mmtk_object_reference_write_pre(
343343
) {
344344
mutator
345345
.barrier()
346-
.object_reference_write_pre(src, slot, target);
346+
.object_reference_write_pre(src, slot.into(), target);
347347
}
348348

349349
/// Full post barrier
@@ -356,7 +356,7 @@ pub extern "C" fn mmtk_object_reference_write_post(
356356
) {
357357
mutator
358358
.barrier()
359-
.object_reference_write_post(src, slot, target);
359+
.object_reference_write_post(src, slot.into(), target);
360360
}
361361

362362
/// Barrier slow-path call
@@ -369,7 +369,7 @@ pub extern "C" fn mmtk_object_reference_write_slow(
369369
) {
370370
mutator
371371
.barrier()
372-
.object_reference_write_slow(src, slot, target);
372+
.object_reference_write_slow(src, slot.into(), target);
373373
}
374374

375375
/// Array-copy pre-barrier
@@ -383,7 +383,7 @@ pub extern "C" fn mmtk_array_copy_pre(
383383
let bytes = count << LOG_BYTES_IN_ADDRESS;
384384
mutator
385385
.barrier()
386-
.memory_region_copy_pre(src..src + bytes, dst..dst + bytes);
386+
.memory_region_copy_pre((src..src + bytes).into(), (dst..dst + bytes).into());
387387
}
388388

389389
/// Array-copy post-barrier
@@ -397,7 +397,7 @@ pub extern "C" fn mmtk_array_copy_post(
397397
let bytes = count << LOG_BYTES_IN_ADDRESS;
398398
mutator
399399
.barrier()
400-
.memory_region_copy_post(src..src + bytes, dst..dst + bytes);
400+
.memory_region_copy_post((src..src + bytes).into(), (dst..dst + bytes).into());
401401
}
402402

403403
/// C2 Slowpath allocation barrier

mmtk/src/gc_work.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl<F: RootsWorkFactory<OpenJDKEdge>> GCWork<OpenJDK> for ScanCodeCacheRoots<F>
5959
let mut edges = Vec::with_capacity(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed));
6060
for roots in (*crate::CODE_CACHE_ROOTS.lock().unwrap()).values() {
6161
for r in roots {
62-
edges.push(*r)
62+
edges.push((*r).into())
6363
}
6464
}
6565
// Create work packet

mmtk/src/lib.rs

Lines changed: 97 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use libc::{c_char, c_void, uintptr_t};
1111
use mmtk::util::alloc::AllocationError;
1212
use mmtk::util::opaque_pointer::*;
1313
use mmtk::util::{Address, ObjectReference};
14+
use mmtk::vm::edge_shape::{AddressRangeIterator, Edge, MemorySlice};
1415
use mmtk::vm::VMBinding;
1516
use mmtk::{MMTKBuilder, Mutator, MMTK};
1617

@@ -136,11 +137,101 @@ pub static FREE_LIST_ALLOCATOR_SIZE: uintptr_t =
136137
pub struct OpenJDK;
137138

138139
/// The type of edges in OpenJDK.
139-
///
140-
/// TODO: We currently make it an alias to Address to make the change minimal.
141-
/// If we support CompressedOOPs, we should define an enum type to support both
142-
/// compressed and uncompressed OOPs.
143-
pub type OpenJDKEdge = Address;
140+
/// Currently it has the same layout as `Address`, but we override its load and store methods.
141+
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
142+
#[repr(transparent)]
143+
pub struct OpenJDKEdge {
144+
pub addr: Address,
145+
}
146+
147+
impl From<Address> for OpenJDKEdge {
148+
fn from(value: Address) -> Self {
149+
Self { addr: value }
150+
}
151+
}
152+
153+
impl Edge for OpenJDKEdge {
154+
fn load(&self) -> ObjectReference {
155+
if cfg!(any(target_arch = "x86", target_arch = "x86_64")) {
156+
// Workaround: On x86 (including x86_64), machine instructions may contain pointers as
157+
// immediates, and they may be unaligned. It is an undefined behavior in Rust to
158+
// dereference unaligned pointers. We have to explicitly use unaligned memory access
159+
// methods. On x86, ordinary MOV instructions can load and store memory at unaligned
160+
// addresses, so we expect `ptr.read_unaligned()` to have no performance penalty over
161+
// `ptr.read()` if `ptr` is actually aligned.
162+
unsafe {
163+
let ptr = self.addr.to_ptr::<ObjectReference>();
164+
ptr.read_unaligned()
165+
}
166+
} else {
167+
unsafe { self.addr.load() }
168+
}
169+
}
170+
171+
fn store(&self, object: ObjectReference) {
172+
if cfg!(any(target_arch = "x86", target_arch = "x86_64")) {
173+
unsafe {
174+
let ptr = self.addr.to_mut_ptr::<ObjectReference>();
175+
ptr.write_unaligned(object)
176+
}
177+
} else {
178+
unsafe { self.addr.store(object) }
179+
}
180+
}
181+
}
182+
183+
/// A range of OpenJDKEdge, usually used for arrays.
184+
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
185+
pub struct OpenJDKEdgeRange {
186+
range: Range<Address>,
187+
}
188+
189+
impl From<Range<Address>> for OpenJDKEdgeRange {
190+
fn from(value: Range<Address>) -> Self {
191+
Self { range: value }
192+
}
193+
}
194+
195+
pub struct OpenJDKEdgeRangeIterator {
196+
inner: AddressRangeIterator,
197+
}
198+
199+
impl Iterator for OpenJDKEdgeRangeIterator {
200+
type Item = OpenJDKEdge;
201+
202+
fn next(&mut self) -> Option<Self::Item> {
203+
self.inner.next().map(|a| a.into())
204+
}
205+
}
206+
207+
// Note that we cannot implement MemorySlice for `Range<OpenJDKEdgeRange>` because neither
208+
// `MemorySlice` nor `Range<T>` are defined in the `mmtk-openjdk` crate. ("orphan rule")
209+
impl MemorySlice for OpenJDKEdgeRange {
210+
type Edge = OpenJDKEdge;
211+
type EdgeIterator = OpenJDKEdgeRangeIterator;
212+
213+
fn iter_edges(&self) -> Self::EdgeIterator {
214+
OpenJDKEdgeRangeIterator {
215+
inner: self.range.iter_edges(),
216+
}
217+
}
218+
219+
fn object(&self) -> Option<ObjectReference> {
220+
self.range.object()
221+
}
222+
223+
fn start(&self) -> Address {
224+
self.range.start()
225+
}
226+
227+
fn bytes(&self) -> usize {
228+
self.range.bytes()
229+
}
230+
231+
fn copy(src: &Self, tgt: &Self) {
232+
MemorySlice::copy(&src.range, &tgt.range)
233+
}
234+
}
144235

145236
impl VMBinding for OpenJDK {
146237
type VMObjectModel = object_model::VMObjectModel;
@@ -150,7 +241,7 @@ impl VMBinding for OpenJDK {
150241
type VMReferenceGlue = reference_glue::VMReferenceGlue;
151242

152243
type VMEdge = OpenJDKEdge;
153-
type VMMemorySlice = Range<Address>;
244+
type VMMemorySlice = OpenJDKEdgeRange;
154245

155246
const MIN_ALIGNMENT: usize = 8;
156247
const MAX_ALIGNMENT: usize = 8;

mmtk/src/object_scanning.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ impl OopIterate for OopMapBlock {
1616
fn oop_iterate(&self, oop: Oop, closure: &mut impl EdgeVisitor<OpenJDKEdge>) {
1717
let start = oop.get_field_address(self.offset);
1818
for i in 0..self.count as usize {
19-
let edge = start + (i << LOG_BYTES_IN_ADDRESS);
19+
let edge = (start + (i << LOG_BYTES_IN_ADDRESS)).into();
2020
closure.visit_edge(edge);
2121
}
2222
}
@@ -66,7 +66,7 @@ impl OopIterate for InstanceMirrorKlass {
6666
let len = Self::static_oop_field_count(oop);
6767
let slice = unsafe { slice::from_raw_parts(start, len as _) };
6868
for oop in slice {
69-
closure.visit_edge(Address::from_ref(oop as &Oop));
69+
closure.visit_edge(Address::from_ref(oop as &Oop).into());
7070
}
7171
}
7272
}
@@ -88,7 +88,7 @@ impl OopIterate for ObjArrayKlass {
8888
fn oop_iterate(&self, oop: Oop, closure: &mut impl EdgeVisitor<OpenJDKEdge>) {
8989
let array = unsafe { oop.as_array_oop() };
9090
for oop in unsafe { array.data::<Oop>(BasicType::T_OBJECT) } {
91-
closure.visit_edge(Address::from_ref(oop as &Oop));
91+
closure.visit_edge(Address::from_ref(oop as &Oop).into());
9292
}
9393
}
9494
}
@@ -133,9 +133,9 @@ impl InstanceRefKlass {
133133
}
134134
fn process_ref_as_strong(oop: Oop, closure: &mut impl EdgeVisitor<OpenJDKEdge>) {
135135
let referent_addr = Self::referent_address(oop);
136-
closure.visit_edge(referent_addr);
136+
closure.visit_edge(referent_addr.into());
137137
let discovered_addr = Self::discovered_address(oop);
138-
closure.visit_edge(discovered_addr);
138+
closure.visit_edge(discovered_addr.into());
139139
}
140140
}
141141

mmtk/src/scanning.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ extern "C" fn report_edges_and_renew_buffer<F: RootsWorkFactory<OpenJDKEdge>>(
2020
factory_ptr: *mut libc::c_void,
2121
) -> NewBuffer {
2222
if !ptr.is_null() {
23-
let buf = unsafe { Vec::<Address>::from_raw_parts(ptr, length, capacity) };
23+
// Note: Currently OpenJDKEdge has the same layout as Address. If the layout changes, we
24+
// should fix the Rust-to-C interface.
25+
let buf = unsafe { Vec::<OpenJDKEdge>::from_raw_parts(ptr as _, length, capacity) };
2426
let factory: &mut F = unsafe { &mut *(factory_ptr as *mut F) };
2527
factory.create_process_edge_roots_work(buf);
2628
}

0 commit comments

Comments
 (0)