Skip to content

Commit

Permalink
Fix write barrier parameter type (#1130)
Browse files Browse the repository at this point in the history
Change the target type of write barrier functions to
`Option<ObjectReference>`. This allows VM bindings to use the write
barrier API when storing NULL references or non-references values to
slots.

Fixes: #1129
  • Loading branch information
wks authored Apr 30, 2024
1 parent a02803b commit fea59e4
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 18 deletions.
30 changes: 26 additions & 4 deletions src/memory_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,24 @@ pub fn post_alloc<VM: VMBinding>(
/// * `src`: The modified source object.
/// * `slot`: The location of the field to be modified.
/// * `target`: The target for the write operation.
///
/// # Deprecated
///
/// This function needs to be redesigned. Its current form has multiple issues.
///
/// - It is only able to write non-null object references into the slot. But dynamic language
/// VMs may write non-reference values, such as tagged small integers, special values such as
/// `null`, `undefined`, `true`, `false`, etc. into a field that previous contains an object
/// reference.
/// - It relies on `slot.store` to write `target` into the slot, but `slot.store` is designed for
/// forwarding references when an object is moved by GC, and is supposed to preserve tagged
/// type information, the offset (if it is an interior pointer), etc. A write barrier is
/// associated to an assignment operation, which usually updates such information instead.
///
/// We will redesign a more general subsuming write barrier to address those problems and replace
/// the current `object_reference_write`. Before that happens, VM bindings should use
/// `object_reference_write_pre` and `object_reference_write_post` instead.
#[deprecated = "Use `object_reference_write_pre` and `object_reference_write_post` instead, until this function is redesigned"]
pub fn object_reference_write<VM: VMBinding>(
mutator: &mut Mutator<VM>,
src: ObjectReference,
Expand All @@ -252,12 +270,14 @@ pub fn object_reference_write<VM: VMBinding>(
/// * `mutator`: The mutator for the current thread.
/// * `src`: The modified source object.
/// * `slot`: The location of the field to be modified.
/// * `target`: The target for the write operation.
/// * `target`: The target for the write operation. `None` if the slot did not hold an object
/// reference before the write operation. For example, the slot may be holding a `null`
/// reference, a small integer, or special values such as `true`, `false`, `undefined`, etc.
pub fn object_reference_write_pre<VM: VMBinding>(
mutator: &mut Mutator<VM>,
src: ObjectReference,
slot: VM::VMEdge,
target: ObjectReference,
target: Option<ObjectReference>,
) {
mutator
.barrier()
Expand All @@ -278,12 +298,14 @@ pub fn object_reference_write_pre<VM: VMBinding>(
/// * `mutator`: The mutator for the current thread.
/// * `src`: The modified source object.
/// * `slot`: The location of the field to be modified.
/// * `target`: The target for the write operation.
/// * `target`: The target for the write operation. `None` if the slot no longer hold an object
/// reference after the write operation. This may happen when writing a `null` reference, a small
/// integers, or a special value such as`true`, `false`, `undefined`, etc., into the slot.
pub fn object_reference_write_post<VM: VMBinding>(
mutator: &mut Mutator<VM>,
src: ObjectReference,
slot: VM::VMEdge,
target: ObjectReference,
target: Option<ObjectReference>,
) {
mutator
.barrier()
Expand Down
16 changes: 8 additions & 8 deletions src/plan/barriers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,17 @@ pub trait Barrier<VM: VMBinding>: 'static + Send + Downcast {
slot: VM::VMEdge,
target: ObjectReference,
) {
self.object_reference_write_pre(src, slot, target);
self.object_reference_write_pre(src, slot, Some(target));
slot.store(target);
self.object_reference_write_post(src, slot, target);
self.object_reference_write_post(src, slot, Some(target));
}

/// Full pre-barrier for object reference write
fn object_reference_write_pre(
&mut self,
_src: ObjectReference,
_slot: VM::VMEdge,
_target: ObjectReference,
_target: Option<ObjectReference>,
) {
}

Expand All @@ -71,7 +71,7 @@ pub trait Barrier<VM: VMBinding>: 'static + Send + Downcast {
&mut self,
_src: ObjectReference,
_slot: VM::VMEdge,
_target: ObjectReference,
_target: Option<ObjectReference>,
) {
}

Expand All @@ -81,7 +81,7 @@ pub trait Barrier<VM: VMBinding>: 'static + Send + Downcast {
&mut self,
_src: ObjectReference,
_slot: VM::VMEdge,
_target: ObjectReference,
_target: Option<ObjectReference>,
) {
}

Expand Down Expand Up @@ -147,7 +147,7 @@ pub trait BarrierSemantics: 'static + Send {
&mut self,
src: ObjectReference,
slot: <Self::VM as VMBinding>::VMEdge,
target: ObjectReference,
target: Option<ObjectReference>,
);

/// Slow-path call for mempry slice copy operations. For example, array-copy operations.
Expand Down Expand Up @@ -217,7 +217,7 @@ impl<S: BarrierSemantics> Barrier<S::VM> for ObjectBarrier<S> {
&mut self,
src: ObjectReference,
slot: <S::VM as VMBinding>::VMEdge,
target: ObjectReference,
target: Option<ObjectReference>,
) {
if self.object_is_unlogged(src) {
self.object_reference_write_slow(src, slot, target);
Expand All @@ -228,7 +228,7 @@ impl<S: BarrierSemantics> Barrier<S::VM> for ObjectBarrier<S> {
&mut self,
src: ObjectReference,
slot: <S::VM as VMBinding>::VMEdge,
target: ObjectReference,
target: Option<ObjectReference>,
) {
if self.log_object(src) {
self.semantics
Expand Down
2 changes: 1 addition & 1 deletion src/plan/generational/barrier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<VM: VMBinding, P: GenerationalPlanExt<VM> + PlanTraceObject<VM>> BarrierSem
&mut self,
src: ObjectReference,
_slot: VM::VMEdge,
_target: ObjectReference,
_target: Option<ObjectReference>,
) {
// enqueue the object
self.modbuf.push(src);
Expand Down
11 changes: 6 additions & 5 deletions src/vm/tests/mock_tests/mock_test_barrier_slow_path_assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn test_assertion_barrier_invalid_ref() {
fixture.mutator_mut().barrier.object_reference_write_slow(
invalid_objref,
edge,
objref,
Some(objref),
);
});
},
Expand All @@ -51,10 +51,11 @@ fn test_assertion_barrier_valid_ref() {
let edge = Address::from_ref(&slot);

// Invoke barrier slowpath with the valid object ref
fixture
.mutator_mut()
.barrier
.object_reference_write_slow(objref, edge, objref);
fixture.mutator_mut().barrier.object_reference_write_slow(
objref,
edge,
Some(objref),
);
});
},
no_cleanup,
Expand Down

0 comments on commit fea59e4

Please sign in to comment.