8241613: Suspicious calls to MacroAssembler::null_check(Register, offset)

Reviewed-by: dholmes, coleenp, fparain, adinn
This commit is contained in:
Matias Saavedra Silva 2023-04-03 12:59:02 +00:00
parent 33d09e587a
commit 127afd3445
22 changed files with 28 additions and 69 deletions

@ -4323,11 +4323,6 @@ void MacroAssembler::load_klass(Register dst, Register src) {
}
}
void MacroAssembler::load_klass_check_null(Register dst, Register src) {
null_check(src, oopDesc::klass_offset_in_bytes());
load_klass(dst, src);
}
// ((OopHandle)result).resolve();
void MacroAssembler::resolve_oop_handle(Register result, Register tmp1, Register tmp2) {
// OopHandle::resolve is an indirection.

@ -850,7 +850,6 @@ public:
// oop manipulations
void load_klass(Register dst, Register src);
void load_klass_check_null(Register dst, Register src);
void store_klass(Register dst, Register src);
void cmp_klass(Register oop, Register trial_klass, Register tmp);

@ -322,7 +322,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
__ null_check(receiver_reg);
} else {
// load receiver klass itself
__ load_klass_check_null(temp1_recv_klass, receiver_reg);
__ load_klass(temp1_recv_klass, receiver_reg);
__ verify_klass_ptr(temp1_recv_klass);
}
BLOCK_COMMENT("check_receiver {");

@ -730,8 +730,6 @@ void TemplateTable::wide_aload()
void TemplateTable::index_check(Register array, Register index)
{
// destroys r1, rscratch1
// check array
__ null_check(array, arrayOopDesc::length_offset_in_bytes());
// sign extend index for use by indexed load
// __ movl2ptr(index, index);
// check index
@ -3305,7 +3303,7 @@ void TemplateTable::invokevirtual_helper(Register index,
__ bind(notFinal);
// get receiver klass
__ load_klass_check_null(r0, recv);
__ load_klass(r0, recv);
// profile this call
__ profile_virtual_call(r0, rlocals, r3);
@ -3393,8 +3391,8 @@ void TemplateTable::invokeinterface(int byte_no) {
Label notVFinal;
__ tbz(r3, ConstantPoolCacheEntry::is_vfinal_shift, notVFinal);
// Get receiver klass into r3 - also a null check
__ load_klass_check_null(r3, r2);
// Get receiver klass into r3
__ load_klass(r3, r2);
Label subtype;
__ check_klass_subtype(r3, r0, r4, subtype);
@ -3408,9 +3406,9 @@ void TemplateTable::invokeinterface(int byte_no) {
__ bind(notVFinal);
// Get receiver klass into r3 - also a null check
// Get receiver klass into r3
__ restore_locals();
__ load_klass_check_null(r3, r2);
__ load_klass(r3, r2);
Label no_such_method;
@ -3650,7 +3648,6 @@ void TemplateTable::anewarray() {
void TemplateTable::arraylength() {
transition(atos, itos);
__ null_check(r0, arrayOopDesc::length_offset_in_bytes());
__ ldrw(r0, Address(r0, arrayOopDesc::length_offset_in_bytes()));
}

@ -1653,11 +1653,6 @@ void MacroAssembler::load_klass(Register dst_klass, Register src_oop, AsmConditi
ldr(dst_klass, Address(src_oop, oopDesc::klass_offset_in_bytes()), cond);
}
void MacroAssembler::load_klass_check_null(Register dst_klass, Register src_oop, Register tmp, AsmCondition cond) {
null_check(src_oop, tmp, oopDesc::klass_offset_in_bytes());
load_klass(dst_klass, src_oop, cond);
}
// Blows src_klass.
void MacroAssembler::store_klass(Register src_klass, Register dst_oop) {
str(src_klass, Address(dst_oop, oopDesc::klass_offset_in_bytes()));

@ -853,7 +853,6 @@ public:
// klass oop manipulations if compressed
void load_klass(Register dst_klass, Register src_oop, AsmCondition cond = al);
void load_klass_check_null(Register dst_klass, Register src_oop, Register tmp, AsmCondition cond = al);
void store_klass(Register src_klass, Register dst_oop);

@ -331,7 +331,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
__ null_check(receiver_reg, temp3);
} else {
// load receiver klass itself
__ load_klass_check_null(temp1_recv_klass, receiver_reg, temp3);
__ load_klass(temp1_recv_klass, receiver_reg);
__ verify_klass_ptr(temp1_recv_klass);
}
BLOCK_COMMENT("check_receiver {");

@ -812,8 +812,6 @@ void TemplateTable::index_check(Register array, Register index) {
void TemplateTable::index_check_without_pop(Register array, Register index) {
assert_different_registers(array, index, Rtemp);
// check array
__ null_check(array, Rtemp, arrayOopDesc::length_offset_in_bytes());
// check index
__ ldr_s32(Rtemp, Address(array, arrayOopDesc::length_offset_in_bytes()));
__ cmp_32(index, Rtemp);
@ -3625,7 +3623,7 @@ void TemplateTable::invokevirtual_helper(Register index,
__ bind(notFinal);
// get receiver klass
__ load_klass_check_null(recv_klass, recv, Rtemp);
__ load_klass(recv_klass, recv);
// profile this call
__ profile_virtual_call(R0_tmp, recv_klass);
@ -4004,7 +4002,6 @@ void TemplateTable::anewarray() {
void TemplateTable::arraylength() {
transition(atos, itos);
__ null_check(R0_tos, Rtemp, arrayOopDesc::length_offset_in_bytes());
__ ldr_s32(R0_tos, Address(R0_tos, arrayOopDesc::length_offset_in_bytes()));
}

@ -2080,11 +2080,6 @@ void MacroAssembler::load_klass(Register dst, Register src, Register tmp) {
}
}
void MacroAssembler::load_klass_check_null(Register dst, Register src, Register tmp) {
null_check(src, oopDesc::klass_offset_in_bytes());
load_klass(dst, src, tmp);
}
void MacroAssembler::store_klass(Register dst, Register src, Register tmp) {
// FIXME: Should this be a store release? concurrent gcs assumes
// klass length is valid if klass field is not null.

@ -195,7 +195,6 @@ class MacroAssembler: public Assembler {
void access_store_at(BasicType type, DecoratorSet decorators, Address dst,
Register val, Register tmp1, Register tmp2, Register tmp3);
void load_klass(Register dst, Register src, Register tmp = t0);
void load_klass_check_null(Register dst, Register src, Register tmp = t0);
void store_klass(Register dst, Register src, Register tmp = t0);
void cmp_klass(Register oop, Register trial_klass, Register tmp1, Register tmp2, Label &L);

@ -315,7 +315,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
__ null_check(receiver_reg);
} else {
// load receiver klass itself
__ load_klass_check_null(temp1_recv_klass, receiver_reg);
__ load_klass(temp1_recv_klass, receiver_reg);
__ verify_klass_ptr(temp1_recv_klass);
}
BLOCK_COMMENT("check_receiver {");

@ -688,8 +688,6 @@ void TemplateTable::wide_aload() {
void TemplateTable::index_check(Register array, Register index) {
// destroys x11, t0
// check array
__ null_check(array, arrayOopDesc::length_offset_in_bytes());
// sign extend index for use by indexed load
// check index
const Register length = t0;
@ -3220,7 +3218,7 @@ void TemplateTable::invokevirtual_helper(Register index,
__ bind(notFinal);
// get receiver klass
__ load_klass_check_null(x10, recv);
__ load_klass(x10, recv);
// profile this call
__ profile_virtual_call(x10, xlocals, x13);
@ -3304,8 +3302,8 @@ void TemplateTable::invokeinterface(int byte_no) {
__ andi(t0, x13, 1UL << ConstantPoolCacheEntry::is_vfinal_shift);
__ beqz(t0, notVFinal);
// Check receiver klass into x13 - also a null check
__ load_klass_check_null(x13, x12);
// Check receiver klass into x13
__ load_klass(x13, x12);
Label subtype;
__ check_klass_subtype(x13, x10, x14, subtype);
@ -3319,9 +3317,9 @@ void TemplateTable::invokeinterface(int byte_no) {
__ bind(notVFinal);
// Get receiver klass into x13 - also a null check
// Get receiver klass into x13
__ restore_locals();
__ load_klass_check_null(x13, x12);
__ load_klass(x13, x12);
Label no_such_method;
@ -3558,7 +3556,6 @@ void TemplateTable::anewarray() {
void TemplateTable::arraylength() {
transition(atos, itos);
__ null_check(x10, arrayOopDesc::length_offset_in_bytes());
__ lwu(x10, Address(x10, arrayOopDesc::length_offset_in_bytes()));
}

@ -3651,11 +3651,6 @@ void MacroAssembler::load_klass(Register klass, Register src_oop) {
}
}
void MacroAssembler::load_klass_check_null(Register klass, Register src_oop, Register tmp) {
null_check(src_oop, tmp, oopDesc::klass_offset_in_bytes());
load_klass(klass, src_oop);
}
void MacroAssembler::store_klass(Register klass, Register dst_oop, Register ck) {
if (UseCompressedClassPointers) {
assert_different_registers(dst_oop, klass, Z_R0);

@ -766,7 +766,6 @@ class MacroAssembler: public Assembler {
void decode_klass_not_null(Register dst);
void load_klass(Register klass, Address mem);
void load_klass(Register klass, Register src_oop);
void load_klass_check_null(Register klass, Register src_oop, Register tmp);
void store_klass(Register klass, Register dst_oop, Register ck = noreg); // Klass will get compressed if ck not provided.
void store_klass_gap(Register s, Register dst_oop);

@ -411,7 +411,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
__ null_check(receiver_reg);
} else {
// Load receiver klass itself.
__ load_klass_check_null(temp1_recv_klass, receiver_reg, Z_R0);
__ load_klass(temp1_recv_klass, receiver_reg);
__ verify_klass_ptr(temp1_recv_klass);
}
BLOCK_COMMENT("check_receiver {");

@ -774,9 +774,6 @@ void TemplateTable::wide_aload() {
void TemplateTable::index_check(Register array, Register index, unsigned int shift) {
assert_different_registers(Z_R1_scratch, array, index);
// Check array.
__ null_check(array, Z_R0_scratch, arrayOopDesc::length_offset_in_bytes());
// Sign extend index for use by indexed load.
__ z_lgfr(index, index);
@ -3549,7 +3546,7 @@ void TemplateTable::invokevirtual_helper(Register index,
__ bind(notFinal);
// Get receiver klass.
__ load_klass_check_null(Z_tmp_2, recv, Z_R0_scratch);
__ load_klass(Z_tmp_2, recv);
// Profile this call.
__ profile_virtual_call(Z_tmp_2, Z_ARG4, Z_ARG5);
@ -3924,8 +3921,6 @@ void TemplateTable::arraylength() {
transition(atos, itos);
int offset = arrayOopDesc::length_offset_in_bytes();
__ null_check(Z_tos, Z_R0_scratch, offset);
__ mem2reg_opt(Z_tos, Address(Z_tos, offset), false);
}

@ -84,7 +84,7 @@ VtableStub* VtableStubs::create_vtable_stub(int vtable_index) {
const Register rcvr_klass = Z_R1_scratch;
address npe_addr = __ pc(); // npe == NULL ptr exception
// Get receiver klass.
__ load_klass_check_null(rcvr_klass, Z_ARG1, Z_R1_scratch);
__ load_klass(rcvr_klass, Z_ARG1);
#ifndef PRODUCT
if (DebugVtables) {
@ -194,7 +194,7 @@ VtableStub* VtableStubs::create_itable_stub(int itable_index) {
// Get receiver klass.
// Must do an explicit check if offset too large or implicit checks are disabled.
address npe_addr = __ pc(); // npe == NULL ptr exception
__ load_klass_check_null(rcvr_klass, Z_ARG1, Z_R1_scratch);
__ load_klass(rcvr_klass, Z_ARG1);
// Receiver subtype check against REFC.
__ z_lg(interface, Address(Z_method, CompiledICHolder::holder_klass_offset()));

@ -5131,11 +5131,6 @@ void MacroAssembler::load_klass(Register dst, Register src, Register tmp) {
movptr(dst, Address(src, oopDesc::klass_offset_in_bytes()));
}
void MacroAssembler::load_klass_check_null(Register dst, Register src, Register tmp) {
null_check(src, oopDesc::klass_offset_in_bytes());
load_klass(dst, src, tmp);
}
void MacroAssembler::store_klass(Register dst, Register src, Register tmp) {
assert_different_registers(src, tmp);
assert_different_registers(dst, tmp);

@ -362,7 +362,6 @@ class MacroAssembler: public Assembler {
// oop manipulations
void load_klass(Register dst, Register src, Register tmp);
void load_klass_check_null(Register dst, Register src, Register tmp);
void store_klass(Register dst, Register src, Register tmp);
void access_load_at(BasicType type, DecoratorSet decorators, Register dst, Address src,

@ -379,7 +379,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
__ null_check(receiver_reg);
} else {
// load receiver klass itself
__ load_klass_check_null(temp1_recv_klass, receiver_reg, temp2);
__ load_klass(temp1_recv_klass, receiver_reg, temp2);
__ verify_klass_ptr(temp1_recv_klass);
}
BLOCK_COMMENT("check_receiver {");

@ -750,8 +750,6 @@ void TemplateTable::index_check(Register array, Register index) {
void TemplateTable::index_check_without_pop(Register array, Register index) {
// destroys rbx
// check array
__ null_check(array, arrayOopDesc::length_offset_in_bytes());
// sign extend index for use by indexed load
__ movl2ptr(index, index);
// check index
@ -3729,7 +3727,7 @@ void TemplateTable::invokevirtual_helper(Register index,
__ bind(notFinal);
// get receiver klass
__ load_klass_check_null(rax, recv, rscratch1);
__ load_klass(rax, recv, rscratch1);
// profile this call
__ profile_virtual_call(rax, rlocals, rdx);
@ -3820,7 +3818,7 @@ void TemplateTable::invokeinterface(int byte_no) {
__ jcc(Assembler::zero, notVFinal);
// Get receiver klass into rlocals - also a null check
__ load_klass_check_null(rlocals, rcx, rscratch1);
__ load_klass(rlocals, rcx, rscratch1);
Label subtype;
__ check_klass_subtype(rlocals, rax, rbcp, subtype);
@ -3842,7 +3840,7 @@ void TemplateTable::invokeinterface(int byte_no) {
// Get receiver klass into rdx - also a null check
__ restore_locals(); // restore r14
__ load_klass_check_null(rdx, rcx, rscratch1);
__ load_klass(rdx, rcx, rscratch1);
Label no_such_method;
@ -4123,7 +4121,6 @@ void TemplateTable::anewarray() {
void TemplateTable::arraylength() {
transition(atos, itos);
__ null_check(rax, arrayOopDesc::length_offset_in_bytes());
__ movl(rax, Address(rax, arrayOopDesc::length_offset_in_bytes()));
}

@ -318,6 +318,12 @@ void Universe::genesis(TRAPS) {
ResourceMark rm(THREAD);
HandleMark hm(THREAD);
// Explicit null checks are needed if these offsets are not smaller than the page size
assert(oopDesc::klass_offset_in_bytes() < static_cast<intptr_t>(os::vm_page_size()),
"Klass offset is expected to be less than the page size");
assert(arrayOopDesc::length_offset_in_bytes() < static_cast<intptr_t>(os::vm_page_size()),
"Array length offset is expected to be less than the page size");
{ AutoModifyRestore<bool> temporarily(_bootstrapping, true);
{ MutexLocker mc(THREAD, Compile_lock);