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

[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_aload_0      , "nofast_aload_0"      , "b"    , NULL    , 
T_ILLEGAL,  1, true , _aload_0        );
   def(_nofast_iload        , "nofast_iload"        , "bi"   , NULL    , 
T_ILLEGAL,  1, false, _iload          );

So looks like the result type in hotspot's bytecodes.cpp also needs to 
be changed ?

Thanks,
Jini.


On 5/8/2018 11:43 PM, Chris Plummer wrote:
> 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 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 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 displayed.
>>>> JDK-8202693 (SA: clhsdb printall only displays the base
>>>> bytecode for iload) has been filed for this. I would add the
>>>> test for nofast_iload along with the fix for JDK-8202693.
>>>> 
>>>> The modified webrev is at:
>>>> 
>>>> http://cr.openjdk.java.net/~jgeorge/8174995/webrev.01/
>>>> 
>>>> Thanks, Jini.
>>>> 
>>>> On 4/27/2018 1:54 AM, Ioi Lam wrote:
>>>>> 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 _nofast_iload
>>>>> 
>>>>> 
>>>>> Thanks - Ioi
>>>>> 
>>>>> On 4/26/18 11:15 AM, Jini George wrote:
>>>>>> 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.
>>>>>> 
>>>>>> Root cause and proposed fix: SA has been unaware of the new
>>>>>>  bytecodes introduced for rewriting at CDS dump time
>>>>>> (_nofast* bytecodes). The fix is to make SA aware of these
>>>>>> new _nofast* bytecodes.
>>>>>> 
>>>>>> Tests Run and Passed: SA tests on Mach5 (including the
>>>>>> tests modified to test this fix).
>>>>>> 
>>>>>> Thank you, Jini.
>>>>> 
>>> 
> 
>