RFR(XS): 8232005: [s390] More exception checks missing in interpreter

Doerr, Martin martin.doerr at sap.com
Thu Oct 24 15:02:04 UTC 2019


Hi Götz,

thanks for the review.
Right, the MacroAssember_ppc version of call_VM can't check for exceptions. The interpreter version also saves & restores the interpreter state, which is redundant at this place, but it doesn't disturb.
Pushed.

Best regards,
Martin


> -----Original Message-----
> From: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Sent: Donnerstag, 24. Oktober 2019 16:33
> To: Doerr, Martin <martin.doerr at sap.com>; Reingruber, Richard
> <richard.reingruber at sap.com>; hotspot-runtime-dev at openjdk.java.net;
> Schmidt, Lutz <lutz.schmidt at sap.com>; Langer, Christoph
> <christoph.langer at sap.com>
> Subject: RE: RFR(XS): 8232005: [s390] More exception checks missing in
> interpreter
> 
> Hi Martin,
> 
> Checking for additional exception looks good.
> I see you changed the call to MacroAssember in
> templateInterpreterGenerator_ppc.cpp
> to a call to interpreterMacroAssembler. Also you removed a "true", probably
> to clean up the code. Fine too.
> I didn't check for more sites where exceptions need to be checked.
> 
> Thanks,
>   Goetz.
> 
> > -----Original Message-----
> > From: hotspot-runtime-dev <hotspot-runtime-dev-
> bounces at openjdk.java.net>
> > On Behalf Of Doerr, Martin
> > Sent: Mittwoch, 16. Oktober 2019 11:45
> > To: Reingruber, Richard <richard.reingruber at sap.com>; hotspot-runtime-
> > dev at openjdk.java.net; Schmidt, Lutz <lutz.schmidt at sap.com>; Langer,
> > Christoph <christoph.langer at sap.com>
> > Subject: [CAUTION] RE: RFR(XS): 8232005: [s390] More exception checks
> > missing in interpreter
> >
> > Hi Richard,
> >
> > thanks a lot. I have come to the same conclusions.
> >
> > Here's the new webrev:
> >
> http://cr.openjdk.java.net/~mdoerr/8232005_s390_check_exceptions/webr
> ev.
> > 01/
> >
> > Best regards,
> > Martin
> >
> >
> > > -----Original Message-----
> > > From: Reingruber, Richard <richard.reingruber at sap.com>
> > > Sent: Mittwoch, 16. Oktober 2019 10:05
> > > To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> > > dev at openjdk.java.net; Schmidt, Lutz <lutz.schmidt at sap.com>; Langer,
> > > Christoph <christoph.langer at sap.com>
> > > Subject: RE: RFR(XS): 8232005: [s390] More exception checks missing in
> > > interpreter
> > >
> > > Hi Martin
> > >
> > >   > Regarding the s390 code, I think the additional places you mentioned
> are
> > > no bugs.
> > >   > Monitorenter/exit use JRT_ENTRY_NO_ASYNC, so no async exception
> will
> > > get installed.
> > >
> > > You're right. I think InterpreterRuntime::monitorexit can return an
> > > IllegalMonitorStateException,
> > > though, if the object is already unlocked. Would be good if you could add
> the
> > > exception check there.
> > >
> > > Cheers, Richard.
> > >
> > > -----Original Message-----
> > > From: Doerr, Martin <martin.doerr at sap.com>
> > > Sent: Montag, 14. Oktober 2019 19:15
> > > To: Reingruber, Richard <richard.reingruber at sap.com>; hotspot-
> runtime-
> > > dev at openjdk.java.net; Schmidt, Lutz <lutz.schmidt at sap.com>; Langer,
> > > Christoph <christoph.langer at sap.com>
> > > Subject: RE: RFR(XS): 8232005: [s390] More exception checks missing in
> > > interpreter
> > >
> > > Hi Richard,
> > >
> > > thank you for reviewing and for checking other places, too.
> > >
> > > Regarding the s390 code, I think the additional places you mentioned are
> no
> > > bugs.
> > > Monitorenter/exit use JRT_ENTRY_NO_ASYNC, so no async exception
> will
> > > get installed.
> > > I guess the s390 code was designed this way by intention? No idea why
> x86
> > > has exception checks, here.
> > >
> > > It would also be safe to omit the exception check at places, in which we
> > > already have a pending exception which is handled by specific code later
> on.
> > > Async exceptions will get postponed in this case. But this doesn't seem to
> be
> > > the case for the places you mentioned.
> > >
> > > Regarding the PPC64 code, I think we should fix the places you found.
> They
> > > seem to be real bugs AFAICS.
> > >
> > > Best regards,
> > > Martin
> > >
> > >
> > > > -----Original Message-----
> > > > From: Reingruber, Richard <richard.reingruber at sap.com>
> > > > Sent: Freitag, 11. Oktober 2019 11:36
> > > > To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> > > > dev at openjdk.java.net; Schmidt, Lutz <lutz.schmidt at sap.com>;
> Langer,
> > > > Christoph <christoph.langer at sap.com>
> > > > Subject: RE: RFR(XS): 8232005: [s390] More exception checks missing in
> > > > interpreter
> > > >
> > > > Hi Martin,
> > > >
> > > > Thanks for analyzing and fixing those.
> > > >
> > > > Have you looked at the following:
> > > >
> > > > cpu/s390/interp_masm_s390.cpp:970:    call_VM(noreg,
> > > > CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorenter),
> > > > cpu/s390/interp_masm_s390.cpp-971-            monitor,
> > > > /*check_for_exceptions=*/false);
> > > > cpu/s390/interp_masm_s390.cpp:1150:  call_VM(noreg,
> > > > CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit),
> > > > cpu/s390/interp_masm_s390.cpp-1151-          monitor,
> > > > /*check_for_exceptions=*/false);
> > > > cpu/ppc/templateInterpreterGenerator_ppc.cpp:595:    __
> > > > call_VM(Rexception, CAST_FROM_FN_PTR(address,
> > > > InterpreterRuntime::create_klass_exception), false);
> > > > cpu/ppc/templateInterpreterGenerator_ppc.cpp:598:    __
> > > > call_VM(Rexception, CAST_FROM_FN_PTR(address,
> > > > InterpreterRuntime::create_exception), false);
> > > > cpu/ppc/templateInterpreterGenerator_ppc.cpp:2108:    __
> > > > MacroAssembler::call_VM(R4_ARG2, CAST_FROM_FN_PTR(address,
> > > > InterpreterRuntime::member_name_arg_or_null), R4_ARG2,
> > > R19_method,
> > > > R14_bcp, false);
> > > >
> > > > x86 does the exception check there. I think the check is needed,
> because
> > > > async. exceptions can get installed there and if we don't check, they
> could
> > > be
> > > > dropped.
> > > >
> > > > Thanks, Richard.
> > > > (Not yet Reviewer)
> > > >
> > > > -----Original Message-----
> > > > From: hotspot-runtime-dev <hotspot-runtime-dev-
> > > > bounces at openjdk.java.net> On Behalf Of Doerr, Martin
> > > > Sent: Dienstag, 8. Oktober 2019 16:42
> > > > To: hotspot-runtime-dev at openjdk.java.net; Schmidt, Lutz
> > > > <lutz.schmidt at sap.com>; Langer, Christoph
> <christoph.langer at sap.com>
> > > > Subject: [CAUTION] RFR(XS): 8232005: [s390] More exception checks
> > > missing
> > > > in interpreter
> > > >
> > > > Hi,
> > > >
> > > > the template interpreter on s390 misses more exception checks when
> > > calling
> > > > VM functions which may install an async exception (like
> > > > java.lang.ThreadDeath) at a safepoint.
> > > >
> > > > Bug:
> > > > https://bugs.openjdk.java.net/browse/JDK-8232005
> > > >
> > > > These are the two places I found:
> > > >
> > >
> http://cr.openjdk.java.net/~mdoerr/8232005_s390_check_exceptions/webr
> > > > ev.00/
> > > >
> > > > Please review.
> > > >
> > > > Best regards,
> > > > Martin



More information about the hotspot-runtime-dev mailing list