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

[David Holmes]

On 27/05/2016 7:51 AM, Artem Smotrakov wrote:
> 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):

That would either be a very old compiler or else we're not explicitly 
enabling the right C version and/or GNU extensions.

Cheers,
David

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