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

[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 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.
>>>>>>
>>>>
>>
>>