[aarch64-port-dev ] RFR(L): 8197405: Improve messages of AbstractMethodErrors and IncompatibleClassChangeErrors.

[Lindenmaier, Goetz]

Hi Ningsheng,

thanks for the patch! I added it to my change and enabled the tests on aarch.
New webrev:  
http://cr.openjdk.java.net/~goetz/wr18/8197405-ameExMsg/webrev.07/

Best regards,
  Goetz.

> -----Original Message-----
> From: Ningsheng Jian [mailto:ningsheng.jian at linaro.org]
> Sent: Montag, 5. März 2018 05:28
> To: Andrew Haley <aph at redhat.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; aarch64-port-dev
> <aarch64-port-dev at openjdk.java.net>; Doerr, Martin
> <martin.doerr at sap.com>; David Holmes <david.holmes at oracle.com>;
> hotspot-runtime-dev at openjdk.java.net
> Subject: Re: [aarch64-port-dev ] RFR(L): 8197405: Improve messages of
> AbstractMethodErrors and IncompatibleClassChangeErrors.
> 
> Hi,
> 
> On 2 March 2018 at 16:48, Andrew Haley <aph at redhat.com> wrote:
> > On 02/03/18 01:54, Ningsheng Jian wrote:
> >> Hi Goetz,
> >>
> >> On 28 February 2018 at 19:41, Lindenmaier, Goetz
> >> <goetz.lindenmaier at sap.com> wrote:
> >>> Hi,
> >>>
> >>> The debug build of our nightly test showed an issue of our latest builds.
> >>> Thus new webrev with change in
> throw_AbstractMethodErrorWithMethod()
> >>> http://cr.openjdk.java.net/~goetz/wr18/8197405-
> ameExMsg/webrev.05/
> >>>
> >>> Besides the one problem in my test which is fixed by the new webrev,
> >>> all nightly tests were green.
> >>>
> >>
> >> I am not a reviewer. Just eyeballed the aarch64 changes:
> >>
> >> src/hotspot/cpu/aarch64/vtableStubs_aarch64.cpp:
> >>
> >> 209   __
> far_jump(RuntimeAddress(SharedRuntime::get_handle_wrong_method_st
> ub());
> >>
> >> Another ")" is missing at the end?
> >>
> >> Compared to other ports, the aarch64 invokeinterface should also be
> >> updated. Since you've disabled the tests on aarch64, this can also be
> >> fixed in another patch. CCed aarch64 people.
> >
> > Hi,
> >
> > Since you've already reviewed this, can you please fix it?
> >
> 
> I think adding the following patch should work for AArch64:
> 
> --------
> --- a/src/hotspot/cpu/aarch64/templateTable_aarch64.cpp
> +++ b/src/hotspot/cpu/aarch64/templateTable_aarch64.cpp
> @@ -3440,6 +3440,8 @@ void TemplateTable::invokeinterface(int byte_no) {
> 
>    Label no_such_interface, no_such_method;
> 
> +  // Preserve method for throw_AbstractMethodErrorVerbose.
> +  __ mov(r16, rmethod);
>    // Receiver subtype check against REFC.
>    // Superklass in r0. Subklass in r3. Blows rscratch2, r13
>    __ lookup_interface_method(// inputs: rec. class, interface, itable index
> @@ -3460,8 +3462,10 @@ void TemplateTable::invokeinterface(int byte_no)
> {
>    __ subw(rmethod, rmethod, Method::itable_index_max);
>    __ negw(rmethod, rmethod);
> 
> +  // Preserve recvKlass for throw_AbstractMethodErrorVerbose.
> +  __ mov(rlocals, r3);
>    __ lookup_interface_method(// inputs: rec. class, interface, itable index
> -                             r3, r0, rmethod,
> +                             rlocals, r0, rmethod,
>                               // outputs: method, scan temp. reg
>                               rmethod, r13,
>                               no_such_interface);
> @@ -3490,7 +3494,8 @@ void TemplateTable::invokeinterface(int byte_no) {
>    // throw exception
>    __ restore_bcp();      // bcp must be correct for exception handler
>   (was destroyed)
>    __ restore_locals();   // make sure locals pointer is correct as
> well (was destroyed)
> -  __ call_VM(noreg, CAST_FROM_FN_PTR(address,
> InterpreterRuntime::throw_AbstractMethodError));
> +  // Pass arguments for generating a verbose error message.
> +  __ call_VM(noreg, CAST_FROM_FN_PTR(address,
> InterpreterRuntime::throw_AbstractMethodErrorVerbose), r3, r16);
>    // the call_VM checks for exception, so we should never return here.
>    __ should_not_reach_here();
> 
> @@ -3498,8 +3503,9 @@ void TemplateTable::invokeinterface(int byte_no) {
>    // throw exception
>    __ restore_bcp();      // bcp must be correct for exception handler
>   (was destroyed)
>    __ restore_locals();   // make sure locals pointer is correct as
> well (was destroyed)
> +  // Pass arguments for generating a verbose error message.
>    __ call_VM(noreg, CAST_FROM_FN_PTR(address,
> -                   InterpreterRuntime::throw_IncompatibleClassChangeError));
> +
> InterpreterRuntime::throw_IncompatibleClassChangeErrorVerbose), r3,
> r0);
>    // the call_VM checks for exception, so we should never return here.
> --------
> 
> I enabled Goetz's newly added two cases and they passed on my AArch64
> platform. More regression tests are still running.
> 
> Thanks,
> Ningsheng