8283306: re-resolving indirect call to non-entrant nmethod can crash

Reviewed-by: thartmann, never
This commit is contained in:
Dean Long 2022-05-05 21:28:50 +00:00
parent 4957bc7396
commit 6d7e446162

View File

@ -1817,35 +1817,39 @@ methodHandle SharedRuntime::reresolve_call_site(TRAPS) {
// CLEANUP - with lazy deopt shouldn't need this lock // CLEANUP - with lazy deopt shouldn't need this lock
nmethodLocker nmlock(caller_nm); nmethodLocker nmlock(caller_nm);
// Check relocations for the matching call to 1) avoid false positives,
// and 2) determine the type.
if (call_addr != NULL) { if (call_addr != NULL) {
// On x86 the logic for finding a call instruction is blindly checking for a call opcode 5
// bytes back in the instruction stream so we must also check for reloc info.
RelocIterator iter(caller_nm, call_addr, call_addr+1); RelocIterator iter(caller_nm, call_addr, call_addr+1);
int ret = iter.next(); // Get item bool ret = iter.next(); // Get item
if (ret) { if (ret) {
assert(iter.addr() == call_addr, "must find call"); bool is_static_call = false;
if (iter.type() == relocInfo::static_call_type) { switch (iter.type()) {
is_static_call = true; case relocInfo::static_call_type:
} else { is_static_call = true;
assert(iter.type() == relocInfo::virtual_call_type ||
iter.type() == relocInfo::opt_virtual_call_type
, "unexpected relocInfo. type");
}
} else {
assert(!UseInlineCaches, "relocation info. must exist for this address");
}
// Cleaning the inline cache will force a new resolve. This is more robust case relocInfo::virtual_call_type:
// than directly setting it to the new destination, since resolving of calls case relocInfo::opt_virtual_call_type:
// is always done through the same code path. (experience shows that it // Cleaning the inline cache will force a new resolve. This is more robust
// leads to very hard to track down bugs, if an inline cache gets updated // than directly setting it to the new destination, since resolving of calls
// to a wrong method). It should not be performance critical, since the // is always done through the same code path. (experience shows that it
// resolve is only done once. // leads to very hard to track down bugs, if an inline cache gets updated
// to a wrong method). It should not be performance critical, since the
for (;;) { // resolve is only done once.
ICRefillVerifier ic_refill_verifier; guarantee(iter.addr() == call_addr, "must find call");
if (!clear_ic_at_addr(caller_nm, call_addr, is_static_call)) { for (;;) {
InlineCacheBuffer::refill_ic_stubs(); ICRefillVerifier ic_refill_verifier;
} else { if (!clear_ic_at_addr(caller_nm, call_addr, is_static_call)) {
break; InlineCacheBuffer::refill_ic_stubs();
} else {
break;
}
}
break;
default:
break;
} }
} }
} }