ObjectReference is required to be word aligned. addr: 0x8000026 #291
Comments
|
This issue can also be reproduced on a different machine, and with |
|
This issue is only reproducible when we use a maximum heap size that is larger than 4G, and when compressed oops is enabled. We are not seeing this issue in our CI tests, as we test with heap sizes that are smaller than 4G. So the problem is that after encoding, a compressed oop is not word aligned. The encoding for compressed oops includes right-shifting a word aligned 64-bit pointer by 3 bits, and there seems no guarantee that the compressed oops are word aligned. We cannot directly pass compressed oops to MMTk as |
|
In However, the Rust function is defined as following: pub extern "C" fn mmtk_object_reference_write_slow(
mutator: *mut libc::c_void,
src: ObjectReference,
slot: Address,
target: NullableObjectReference,
) {
with_mutator!(|mutator| {
mutator
.barrier()
.object_reference_write_slow(src, slot.into(), target.into());
})
}This means The way to fix it is introducing a Rust function that includes the decoding pub extern "C" fn mmtk_object_reference_write_slow_compressedoops(
mutator: *mut libc::c_void,
src_compressed: u32,
slot_compressed: u32,
target_compressed: u32,
) {
let src: ObjectReference = decompress(src_compressed);
let slot: OpenJDKSlot<true> = decompress(slot_compressed);
let target: Option<ObjectReference> = decompress(target_compressed);
with_mutator!(|mutator| {
mutator
.barrier()
.object_reference_write_slow(src, slot, target);
})
}And let the C++ part |
|
Does this not mean the write barrier was broken for OpenJDK? As in it was adding incorrect object references to the remset? |
Yes. I wonder why this has been working for so long. |
|
I tried the version right after introducing compressed pointer support (v0.20): a1a8bdf. It 'worked' in a very hacky way. The object reference we get in write barriers are still compressed pointers. If we add an alignment assertion in write barriers, we still see the assertion fail. The object references (compressed pointers) in MMTk are stored in mod buffers and only used for calling the binding to scan the objects during a GC. The binding knows that it is a compressed pointer, and knows how to decode to get klass information to scan the objects. All the methods related with |
|
Note: When using CompressedOops, the heap start address is 0x4000_0000, so 0x800_0026 must be a compressed pointer. After shifting 3 bits to the left, 0x800_0026 becomes 0x4000_0130, and it is in the heap. |
|
Actually the mmtk-openjdk binding calls the write barriers with uncompressed pointers most of the time. I made several copies of the |
|
I also implemented We can see that the "target" value was uncompressed in the pre barrier, but compressed in the post barrier. Why? The real problem is in the implementation of class MMTkBarrierSetAssembler: public BarrierSetAssembler {
// ...
virtual void store_at(MacroAssembler* masm, DecoratorSet decorators, BasicType type, Address dst, Register val, Register tmp1, Register tmp2) override {
if (type == T_OBJECT || type == T_ARRAY) object_reference_write_pre(masm, decorators, dst, val, tmp1, tmp2);
BarrierSetAssembler::store_at(masm, decorators, type, dst, val, tmp1, tmp2);
if (type == T_OBJECT || type == T_ARRAY) object_reference_write_post(masm, decorators, dst, val, tmp1, tmp2);
}
// ...
};And in OpenJDK ( void BarrierSetAssembler::store_at(MacroAssembler* masm, DecoratorSet decorators, BasicType type,
Address dst, Register val, Register tmp1, Register tmp2) {
bool in_heap = (decorators & IN_HEAP) != 0;
// ...
switch (type) {
case T_OBJECT:
case T_ARRAY: {
if (in_heap) {
if (val == noreg) {
// ... storing constant null reference to the field
} else {
#ifdef _LP64
if (UseCompressedOops) {
assert(!dst.uses(val), "not enough registers");
if (is_not_null) {
__ encode_heap_oop_not_null(val);
} else {
__ encode_heap_oop(val);
}
__ movl(dst, val);
} else
#endif
{
__ movptr(dst, val);
}
}
} else {
// ... error handling
}
break;When // Algorithm must match oop.inline.hpp encode_heap_oop.
void MacroAssembler::encode_heap_oop(Register r) {
// ... assertions go here
if (Universe::narrow_oop_base() == NULL) {
// ... handle null oop.
return;
}
testq(r, r);
cmovq(Assembler::equal, r, r12_heapbase);
subq(r, r12_heapbase);
shrq(r, LogMinObjAlignmentInBytes);
}The SolutionKnowing what's going wrong, I can think of three solutions.
fn mmtk_object_reference_write_no_slot_no_target(mutator: Mutator, src: ObjectReference);And use it specifically for the Option (1) has the fewest assumptions, and it is relatively easy to implement @@ -38,9 +38,20 @@ void MMTkObjectBarrierSetRuntime::object_reference_write_post(oop src, oop* slot
#define __ masm->
+void MMTkObjectBarrierSetAssembler::object_reference_write_pre(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const {
+ if (can_remove_barrier(decorators, val, /* skip_const_null */ true)) return;
+ Register obj = dst.base();
+
+ __ push(val);
+ __ push(val);
+}
+
void MMTkObjectBarrierSetAssembler::object_reference_write_post(MacroAssembler* masm, DecoratorSet decorators, Address dst, Register val, Register tmp1, Register tmp2) const {
if (can_remove_barrier(decorators, val, /* skip_const_null */ true)) return;
Register obj = dst.base();
+
+ __ pop(val);
+ __ pop(val);
#if MMTK_ENABLE_BARRIER_FASTPATH
Label done;But the down side is that it adds two Option (2) should perform better because it adds nothing on the fast path. But we now relies on the behavior of But Option (3) should perform even better because it omits the |
`MMTkBarrierSetAssembler::store_at` calls `object_reference_write_post` after calling `BarrierSetAssembler::store_at`. However, `BarrierSetAssembler::store_at` modifies the `val` register to compress its value in place. Consequently, `object_reference_write_post` will see a compressed pointer in the `val` register. This PR makes two changes. Firstly, in `MMTkObjectBarrierSetAssembler::object_reference_write_post`, we simply set both `c_rarg1` and `c_rarg2` to 0 before calling the write barrier slow path. We exploit the semantics of the `ObjectBarrier` that it simply logs the object without looking at the slot or the target. This will fix the [assertion error](#291) because 0 will be interpreted as a `None` of type `Option<ObjectReference>`. Secondly, we add a `bool compensate_val_reg` parameter to `MMTkBarrierSetAssembler::object_reference_write_post` so that if we call it after `BarrierSetAssembler::store_at`, we can give `object_reference_write_post` a chance to decompress the compressed oop in the `val`. This is intended for implementing *other barriers introduced in the future* that may use the `val` register, and keep the developers informed that the `val` register is mutated in `BarrierSetAssembler::store_at`. Fixes: #291 Related PR: #293
Reproduciable with v0.29.0. It is a bit strange that I didn't see the issue before. It would be good if anyone can try reproduce it and see if the issue actually exists.
MMTk OpenJDK: 1fcc9f1 (v0.29.0)
OpenJDK: 28e56ee32525c32c5a88391d0b01f24e5cd16c0f (as recorded in
Cargo.tomlwith the biding commit)Build:
Run:
The text was updated successfully, but these errors were encountered: