RFR (S) 8169867 Method::restore_unshareable_info does not invoke Method::link_method

[Jiangli Zhou]

Hi Ioi,

Looks good.

I also have one suggestion. To make it a little more easier to read, could you please fold the 'else if (_i2i_entry != NULL)’ block starting at line 1039 and ‘if (!is_shared())’ block starting at line 1047 into to one block?
1032   if (is_shared()) {
1033     address entry = Interpreter::entry_for_cds_method(h_method);
1034     assert(entry != NULL && entry == _i2i_entry && entry == _from_interpreted_entry,
1035            "should be correctly set during dump time");
1036     if (adapter() != NULL) {
1037       return;
1038     }
1039   } else if (_i2i_entry != NULL) {
1040     return;
1041   }
1042   assert( _code == NULL, "nothing compiled yet" );
1043 
1044   // Setup interpreter entrypoint
1045   assert(this == h_method(), "wrong h_method()" );
1046 
1047   if (!is_shared()) {
1048     assert(adapter() == NULL, "init'd to NULL");
1049     address entry = Interpreter::entry_for_method(h_method);
1050     assert(entry != NULL, "interpreter entry must be non-null");
1051     // Sets both _i2i_entry and _from_interpreted_entry
1052     set_interpreter_entry(entry);
1053   }
Thanks,
Jiangli

> On Nov 20, 2016, at 10:58 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> https://bugs.openjdk.java.net/browse/JDK-8169867
> http://cr.openjdk.java.net/~iklam/jdk9/8169867_cds_not_calling_link_method.v01/
> 
> Thanks to Tobias for finding the bug. I have done the following
> 
> + integrated Tobias' suggested fix
> + fixed Method::restore_unshareable_info to call Method::link_method
> + added comments and a diagram to illustrate how the CDS method entry
>  trampolines work.
> 
> BTW, I am a little unhappy about the name ConstMethod::_adapter_trampoline.
> It's basically an extra level of indirection to get to the adapter. However.
> The word "trampoline" usually is used for and extra jump in executable code,
> so it may be a little confusing when we use it for a data pointer here.
> 
> Any suggest for a better name?
> 
> 
> Testing:
> [1] I have tested Tobias' TestInterpreterMethodEntries.java class and
>    now it produces the correct assertion. I won't check in this test, though,
>    since it won't assert anymore after Tobias fixes 8169711.
> 
> # after -XX: or in .hotspotrc:  SuppressErrorAt=/method.cpp:1035
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/home/iklam/jdk/ul/hotspot/src/share/vm/oops/method.cpp:1035), pid=16840, tid=16843
> #  assert(entry != __null && entry == _i2i_entry && entry == _from_interpreted_entry) failed:
> #  should be correctly set during dump time
> 
> [2] Ran RBT in fastdebug build for hotspot/test/:hotspot_all,vm.parallel_class_loading,vm.runtime.testlist
>    All tests passed.
> 
> Thanks
> - Ioi
>