RFR (XXS): 8026376: assert(false) failed: DEBUG MESSAGE: exception pc already set

[Christian Thalinger]

On Oct 14, 2013, at 2:59 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> For the record. Christian and I discussed it and decided to remove 'exception pc' load in deopt_blob on all platforms in separate changes. First, it is broken in x86 code anyway (it is not saved in all paths in deopt_blob). Second, we return into Interpreter which know nothing (and don't care) about compiled code and it does not use this pc data.

Right.  So the fix is:

diff -r f50418dfb1b7 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
--- a/src/cpu/sparc/vm/sharedRuntime_sparc.cpp	Sun Oct 13 13:22:24 2013 -0700
+++ b/src/cpu/sparc/vm/sharedRuntime_sparc.cpp	Mon Oct 14 17:35:02 2013 -0700
@@ -3581,6 +3581,7 @@ void SharedRuntime::generate_deopt_blob(
   // the pending exception will be picked up the interpreter.
   __ ld_ptr(G2_thread, in_bytes(JavaThread::exception_oop_offset()), Oexception);
   __ st_ptr(G0, G2_thread, in_bytes(JavaThread::exception_oop_offset()));
+  __ st_ptr(G0, G2_thread, in_bytes(JavaThread::exception_pc_offset()));
   __ bind(noException);
 
   // deallocate the deoptimization frame taking care to preserve the return values

Will file a bug for removing the load on all architectures.

> 
> Thanks,
> Vladimir
> 
> On 10/14/13 2:17 PM, Christian Thalinger wrote:
>> The reason I've added it is because we do it on x86 but there are comments like this:
>> 
>>   // I think this is useless (throwing pc?)
>>   __ movptr(rdx, Address(rsp, RegisterSaver::rdx_offset_in_bytes()));
>> 
>> So we either should read it everywhere or don't.
>> 
>> On Oct 14, 2013, at 11:43 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 
>>> Don't read exception pc since it is not needed. You may trash a value in that register.
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 10/14/13 11:19 AM, Christian Thalinger wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8026376
>>>> http://cr.openjdk.java.net/~twisti/8026376/webrev.00/
>>>> 
>>>> 8026376: assert(false) failed: DEBUG MESSAGE: exception pc already set
>>>> Reviewed-by:
>>>> 
>>>> It made me suspicious that only the second assert fired (the exception pc check) and not the first one (the exception oop check). I checked all the places where JavaThread::exception_oop_offset() was used to clear the exception oop to see if we also clear the exception pc.
>>>> 
>>>> In the deopt blob we have such a case. On x86 we clear both but on SPARC we don't.
>>>> 
>>>> Added the missing null-store instruction makes the test case pass.
>>>> 
>>>> Reading the exception pc is not really required but I've added it for completeness.
>>>> 
>>