RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-04-26 Thread Jini George
Hello all, Please review the following proposed fix for the issue: https://bugs.openjdk.java.net/browse/JDK-8174995 Webrev: http://cr.openjdk.java.net/~jgeorge/8174995/webrev.00/ Issue: Clhsdb commands like 'where -a', 'printall' would throw an illegal code assertion failure when CDS is used.

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-04-26 Thread Ioi Lam
HI Jini, [1] "_nofast_aload" should be "_nofast_aload_0": aload and aload_0 are two different bytecodes. [2] Only the _nofast_aload_0 bytecode is tested. For completeness, do you think it makes sense to add test cases for these other 3 bytecodes?     _nofast_getfield     _nofast_putfield   

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-07 Thread Jini George
Thank you very much, Ioi, for the review and for the clarifications and help provided offline. I have added the checks for _nofast_getfield and _nofast_putfield. SA has a bug due to which for iload, only the base bytecode (iload) gets displayed -- fast_iload and nofast_iload do not get displaye

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-08 Thread Ioi Lam
Looks good. Thanks! - Ioi On 5/7/18 8:38 PM, Jini George wrote: Thank you very much, Ioi, for the review and for the clarifications and help provided offline. I have added the checks for _nofast_getfield and _nofast_putfield. SA has a bug due to which for iload, only the base bytecode (iload

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-08 Thread Jini George
Thanks, Ioi. Could I get one more reviewer to take a look at this ? Thanks, Jini. On 5/8/2018 8:55 PM, Ioi Lam wrote: Looks good. Thanks! - Ioi On 5/7/18 8:38 PM, Jini George wrote: Thank you very much, Ioi, for the review and for the clarifications and help provided offline. I have added t

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-08 Thread Chris Plummer
Hi Jini, Why are _nofast_aload_0 and _nofast_iload using return type BasicType.getTIllegal() when the return type is known? The test changes look good. thanks, Chris On 5/8/18 8:53 AM, Jini George wrote: Thanks, Ioi. Could I get one more reviewer to take a look at this ? Thanks, Jini. On

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Jini George
Thank you very much, Chris, for taking a look. The bytecodes table initialization in SA's Bytecodes.java mimics the bytecodes table initialization in hotspot's Bytecodes::initialize() from bytecodes.cpp. In bytecodes.cpp, we have the initialization of the _nofast* bytecodes thus: def(_nofast_

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Chris Plummer
I'm not too sure how that field ends up getting used. I'd say what's likely in this case is that it's wrong but doesn't matter. Hopefully someone with a better understanding of the use of this field will chime in. Chris On 5/11/18 12:29 AM, Jini George wrote: Thank you very much, Chris, for t

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Ioi Lam
I think all code that looks at the top of stack (e.g., users of BytecodeStream) operate with the original bytecode (_aload_0), not the quicken one (_fast_aload_0 or _nofast_aload_0), so this bug is never discovered. But I think this is still a bug and should be fixed. I filed https://bugs.open

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Chris Plummer
Thanks Ioi. Jini, you can count me as a reviewer for your changes. thanks, Chris On 5/11/18 9:34 AM, Ioi Lam wrote: I think all code that looks at the top of stack (e.g., users of BytecodeStream) operate with the original bytecode (_aload_0), not the quicken one (_fast_aload_0 or _nofast_alo

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Jini George
Thanks, Ioi. - Jini. On 5/11/2018 10:04 PM, Ioi Lam wrote: I think all code that looks at the top of stack (e.g., users of BytecodeStream) operate with the original bytecode (_aload_0), not the quicken one (_fast_aload_0 or _nofast_aload_0), so this bug is never discovered. But I think this i

Re: RFR: (S): SA: clhsdb 'where -a' throws Assertion Failure with illegal code 236 when CDS is used

2018-05-11 Thread Jini George
Thanks, Chris. - Jini. On 5/11/2018 11:26 PM, Chris Plummer wrote: Thanks Ioi. Jini, you can count me as a reviewer for your changes. thanks, Chris On 5/11/18 9:34 AM, Ioi Lam wrote: I think all code that looks at the top of stack (e.g., users of BytecodeStream) operate with the original b