Skip to content

Commit

Permalink
8241613: Suspicious calls to MacroAssembler::null_check(Register, off…
Browse files Browse the repository at this point in the history
…set)

Reviewed-by: dholmes, coleenp, fparain, adinn
  • Loading branch information
Matias Saavedra Silva committed Apr 3, 2023
1 parent 33d09e5 commit 127afd3
Show file tree
Hide file tree
Showing 22 changed files with 28 additions and 69 deletions.
5 changes: 0 additions & 5 deletions src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,6 @@ class MacroAssembler: public Assembler {

// 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);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/aarch64/methodHandles_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
Expand Down
13 changes: 5 additions & 8 deletions src/hotspot/cpu/aarch64/templateTable_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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()));
}

Expand Down
5 changes: 0 additions & 5 deletions src/hotspot/cpu/arm/macroAssembler_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/arm/macroAssembler_arm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,6 @@ class MacroAssembler: public Assembler {
// 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);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/arm/methodHandles_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
Expand Down
5 changes: 1 addition & 4 deletions src/hotspot/cpu/arm/templateTable_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()));
}

Expand Down
5 changes: 0 additions & 5 deletions src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/riscv/macroAssembler_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/riscv/methodHandles_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
Expand Down
13 changes: 5 additions & 8 deletions src/hotspot/cpu/riscv/templateTable_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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()));
}

Expand Down
5 changes: 0 additions & 5 deletions src/hotspot/cpu/s390/macroAssembler_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/s390/macroAssembler_s390.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/s390/methodHandles_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
Expand Down
7 changes: 1 addition & 6 deletions src/hotspot/cpu/s390/templateTable_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/s390/vtableStubs_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()));
Expand Down
5 changes: 0 additions & 5 deletions src/hotspot/cpu/x86/macroAssembler_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/x86/macroAssembler_x86.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/methodHandles_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {");
Expand Down
9 changes: 3 additions & 6 deletions src/hotspot/cpu/x86/templateTable_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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()));
}

Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/memory/universe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

1 comment on commit 127afd3

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.