RFR(T): 8229925: [s390] Exception check missing in interpreter

[Doerr, Martin]

Hi David,

thank you very much for reviewing and for checking the other platforms.
I agree. PPC64 should also perform the exception check. Even though we have never seen this failure on PPC64.
I've updated the issue and the webrev.00 in place.

After looking at InterpreterRuntime::build_method_counters, I wonder why it has been designed as JRT_ENTRY and not as JRT_LEAF. I couldn't find anything which requires the transition ThreadInVMfromJava and the safepoint check when leaving.

Changing it to a leaf call would also be a valid fix (PPC64 example [1] below). What do you think?

Best regards,
Martin



[1] leaf call
diff -r 40006c0ada91 src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
--- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp        Wed Aug 21 00:08:35 2019 +0200
+++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp        Wed Aug 21 14:36:57 2019 +0200
@@ -2264,8 +2264,7 @@
   ld(Rcounters, in_bytes(Method::method_counters_offset()), method);
   cmpdi(CCR0, Rcounters, 0);
   bne(CCR0, has_counters);
-  call_VM(noreg, CAST_FROM_FN_PTR(address,
-                                  InterpreterRuntime::build_method_counters), method, false);
+  call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::build_method_counters), R16_thread, method);
   ld(Rcounters, in_bytes(Method::method_counters_offset()), method);
   cmpdi(CCR0, Rcounters, 0);
   beq(CCR0, skip); // No MethodCounters, OutOfMemory.
diff -r 40006c0ada91 src/hotspot/share/interpreter/interpreterRuntime.cpp
--- a/src/hotspot/share/interpreter/interpreterRuntime.cpp      Wed Aug 21 00:08:35 2019 +0200
+++ b/src/hotspot/share/interpreter/interpreterRuntime.cpp      Wed Aug 21 14:36:57 2019 +0200
@@ -1190,8 +1190,9 @@
   last_frame.set_mdp(new_mdp);
 JRT_END

-JRT_ENTRY(MethodCounters*, InterpreterRuntime::build_method_counters(JavaThread* thread, Method* m))
+JRT_LEAF(MethodCounters*, InterpreterRuntime::build_method_counters(JavaThread* thread, Method* m))
   MethodCounters* mcs = Method::build_method_counters(m, thread);
+  TRAPS = thread;
   if (HAS_PENDING_EXCEPTION) {
     assert((PENDING_EXCEPTION->is_a(SystemDictionary::OutOfMemoryError_klass())), "we expect only an OOM error here");
     CLEAR_PENDING_EXCEPTION;



> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Mittwoch, 21. August 2019 09:31
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR(T): 8229925: [s390] Exception check missing in interpreter
> 
> Hi Martin,
> 
> On 21/08/2019 1:32 am, Doerr, Martin wrote:
> > Hi,
> >
> > we have sporadically seen "assert(!(((ThreadShadow*)__the_thread__)-
> >has_pending_exception())) failed: Should not have any exceptions
> pending" on s390 while running jtreg test
> "vmTestbase/nsk/jvmti/scenarios/capability/CM03/cm03t001".
> > The VM asserts that no exception is pending after a backedge counter
> overflow. The unexpected pending exception found in
> frequency_counter_overflow_inner is an instance of java.lang.ThreadDeath
> which got installed asynchronously by the test via JVMTI StopThread.
> > The VM call to InterpreterRuntime::build_method_counters misses an
> exception check in the s390 implementation. This is necessary for
> asynchronous exceptions which get installed by this test.
> >
> > The explanation may not be trivial, but the proposed fix is:
> >
> http://cr.openjdk.java.net/~mdoerr/8229925_s390_exception_check/webre
> v.00/
> 
> Seems quite reasonable given the explanation, and comparing similar code
> to other platforms.
> 
> PPC64 would seem to have the same bug:
> 
>    call_VM(noreg, CAST_FROM_FN_PTR(address,
> 
> InterpreterRuntime::build_method_counters), method, false);
> 
> Cheers,
> David
> -----
> 
> > Best regards,
> > Martin
> >