Skip to content

8369947: Bytecode rewriting causes Java heap corruption on RISC-V #323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

8369947: Bytecode rewriting causes Java heap corruption on RISC-V #323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/hotspot/cpu/riscv/interp_masm_riscv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,15 @@ void InterpreterMacroAssembler::load_method_entry(Register cache, Register index
}

#ifdef ASSERT
void InterpreterMacroAssembler::verify_field_offset(Register reg) {
// Verify the field offset is not in the header, implicitly checks for 0
Label L;
mv(t0, oopDesc::base_offset_in_bytes());
bge(reg, t0, L);
stop("bad field offset");
bind(L);
}

void InterpreterMacroAssembler::verify_access_flags(Register access_flags, uint32_t flag,
const char* msg, bool stop_by_hit) {
Label L;
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/cpu/riscv/interp_masm_riscv.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@ class InterpreterMacroAssembler: public MacroAssembler {
void load_field_entry(Register cache, Register index, int bcp_offset = 1);
void load_method_entry(Register cache, Register index, int bcp_offset = 1);

void verify_field_offset(Register reg) NOT_DEBUG_RETURN;

#ifdef ASSERT
void verify_access_flags(Register access_flags, uint32_t flag,
const char* msg, bool stop_by_hit = true);
Expand Down
12 changes: 11 additions & 1 deletion src/hotspot/cpu/riscv/templateTable_riscv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ Address TemplateTable::at_bcp(int offset) {
void TemplateTable::patch_bytecode(Bytecodes::Code bc, Register bc_reg,
Register temp_reg, bool load_bc_into_bc_reg /*=true*/,
int byte_no) {
assert_different_registers(bc_reg, temp_reg);
if (!RewriteBytecodes) { return; }
Label L_patch_done;

Expand Down Expand Up @@ -196,7 +197,11 @@ void TemplateTable::patch_bytecode(Bytecodes::Code bc, Register bc_reg,
__ bind(L_okay);
#endif

// patch bytecode
// Patch bytecode with release store to coordinate with ResolvedFieldEntry loads
// in fast bytecode codelets. load_field_entry has a memory barrier that gains
// the needed ordering, together with control dependency on entering the fast codelet
// itself.
__ membar(MacroAssembler::LoadStore | MacroAssembler::StoreStore);
__ sb(bc_reg, at_bcp(0));
__ bind(L_patch_done);
}
Expand Down Expand Up @@ -3017,6 +3022,7 @@ void TemplateTable::fast_storefield(TosState state) {

// X11: field offset, X12: field holder, X13: flags
load_resolved_field_entry(x12, x12, noreg, x11, x13);
__ verify_field_offset(x11);

{
Label notVolatile;
Expand Down Expand Up @@ -3104,6 +3110,8 @@ void TemplateTable::fast_accessfield(TosState state) {
__ load_field_entry(x12, x11);

__ load_sized_value(x11, Address(x12, in_bytes(ResolvedFieldEntry::field_offset_offset())), sizeof(int), true /*is_signed*/);
__ verify_field_offset(x11);

__ load_unsigned_byte(x13, Address(x12, in_bytes(ResolvedFieldEntry::flags_offset())));

// x10: object
Expand Down Expand Up @@ -3159,7 +3167,9 @@ void TemplateTable::fast_xaccess(TosState state) {
__ ld(x10, aaddress(0));
// access constant pool cache
__ load_field_entry(x12, x13, 2);

__ load_sized_value(x11, Address(x12, in_bytes(ResolvedFieldEntry::field_offset_offset())), sizeof(int), true /*is_signed*/);
__ verify_field_offset(x11);

// make sure exception is reported in correct bcp range (getfield is
// next instruction)
Expand Down