RFR(S): 8169711: CDS does not patch entry trampoline if intrinsic method is disabled
ioi.lam at oracle.com
Thu Nov 17 21:31:39 UTC 2016
The interpreter changes look OK to me. I'll defer to Vladimir on his
opinion on the asserts.
On 11/17/16 11:34 AM, Vladimir Kozlov wrote:
> Hi Tobias,
> It is a little inconsistent. CRC32 instrinsics check their flag in
> generate_CRC32* methods.
> May be we should do the same for FMA instead of assert in
> generate_math_entry() return NULL if flag is false.
> On 11/17/16 4:42 AM, Tobias Hartmann wrote:
>> please review the following patch:
>> When dumping metadata with class data sharing (CDS),
>> Method::unlink_method() takes care of removing all entry points of
>> methods that will be shared. The _i2i and _from_interpreted entries
>> are set to the corresponding address in the _cds_entry_table (see
>> AbstractInterpreter::entry_for_cds_method()). This address points to
>> a trampoline in shared space that jumps to the actual (unshared)
>> interpreter method entry at runtime (see
>> Intrinsic methods may have a special interpreter entry (for example,
>> 'Interpreter::java_lang_math_fmaF') and if they are shared, their
>> entry points are set to such a trampoline that is patched at runtime
>> to jump to the interpreter stub containing the intrinsic code.
>> The problem is that if an intrinsic is enabled during dumping but
>> disabled during re-using the shared archive, the trampoline is not
>> patched and therefore still refers to the old stub address that was
>> only valid during dumping. In debug, we hit the "should be correctly
>> set during dump time" assert in Method::link_method() because the
>> method entries are inconsistent. In product, we crash because we jump
>> to an invalid address through the unpatched trampoline. This problem
>> exists with the FMA, CRC32 and CRC32C intrinsics (see regression test).
>> I fixed this by always creating the interpreter method entries for
>> intrinsified methods but replace them with vanilla entries in
>> TemplateInterpreterGenerator::generate_method_entry() if the
>> intrinsic is disabled at runtime. Like this, we patch the trampoline
>> destination address even if the intrinsic is disabled but just
>> execute the Java bytecodes instead of the stub.
>> While testing, I noticed that the assert in Method::link_method() is
>> not always triggered (sometimes we just crash). This is because
>> 1) the "_from_compiled_entry == NULL" check in
>> Method::restore_unshareable_info() is always false and therefore
>> link_method() is not invoked and
>> 2) in Method::link_method() we only execute the check if the adapter
>> (which is shared) was not yet initialized.
>> I filed JDK-8169867  for this because I'm not too familiar with
>> the CDS internals.
>> Tested with regression test, JPRT and RBT (running).
>>  https://bugs.openjdk.java.net/browse/JDK-8169867
More information about the hotspot-dev