[9] RFR: 8152207: Perform array bound checks while getting a length of bytecode instructions

[Artem Smotrakov]

Hi Harold,

Thank you for review. Here is an updated webrev

http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.01/

If I am not missing something, it might return -1 if an instruction 
index is invalid, so that it doesn't need to do anything else then.

I noticed that the following code may fail with "declaration can not 
follow a statement (E_DECLARATION_IN_CODE)" on some gcc versions (I've 
seen this on Solaris):

...
        default: {
             if (instruction < 0 || instruction > JVM_OPC_MAX)
                 return -1;

             /* A length of 0 indicates an error. */
             int length = opcode_length[instruction];
             return (length <= 0) ? -1 : length;
        }
...

So, I just removed the 'length' variable.

Artem

On 05/26/2016 10:36 AM, harold seigel wrote:
> Hi Artem,
>
> The hotspot changes look good.
>
> For the jdk webrev, can you move the new code at lines 1699 and 1700 
> to just inside the 'default:' alternative?
>
> Thanks, Harold
>
>
> On 5/20/2016 4:05 PM, Artem Smotrakov wrote:
>> Hello,
>>
>> Could someone review this fix, please?
>>
>> http://cr.openjdk.java.net/~asmotrak/8152207/hotspot/webrev.01/
>> http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.00/
>>
>> Artem
>>
>> On 05/10/2016 01:49 PM, Artem Smotrakov wrote:
>>> Hi Christian,
>>>
>>> Thank you for review. Here is updated webrev
>>>
>>> http://cr.openjdk.java.net/~asmotrak/8152207/hotspot/webrev.01/
>>>
>>> Artem
>>>
>>> On 05/09/2016 01:24 PM, Christian Thalinger wrote:
>>>>
>>>>> On May 4, 2016, at 1:48 PM, Artem Smotrakov 
>>>>> <artem.smotrakov at oracle.com <mailto:artem.smotrakov at oracle.com>> 
>>>>> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Please review two small patches for jdk and hotspot repos which 
>>>>> add array bound checks to functions which return a length of 
>>>>> bytecode instruction.
>>>>>
>>>>> Please see details in 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8152207
>>>>>
>>>>> http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Easmotrak/8152207/jdk/webrev.00/>
>>>>> http://cr.openjdk.java.net/~asmotrak/8152207/hotspot/webrev.00/
>>>>
>>>> static bool        is_defined     (int  code)    { return 0 <= code 
>>>> && code < number_of_codes && flags(code, false) != 0; }
>>>> + static int length_for (Code code) { return 0 <= code && code < 
>>>> number_of_codes ? _lengths[code] & 0xF : -1; }
>>>> + static int wide_length_for(Code code) { return 0 <= code && code 
>>>> < number_of_codes ? _lengths[code] >> 4 : -1; }
>>>> You should factor the bound check into a separate method.
>>>>
>>>>>
>>>>> Artem
>>>>
>>>
>>
>