RFR: 8004823: Add VM support for type annotation reflection

[Joel Borggrén-Franck]

Thanks for the review John!

I had this pushed before I saw this. I'll make sure to implement the checks when I start to implement retransform for  type annotations in the VM.

cheers
/Joel

On Dec 20, 2012, at 10:10 PM, John Rose <john.r.rose at oracle.com> wrote:

> This is good work.  You can count me as a reviewer.
> 
> One comment:  There are a couple of places where a field index is used as an annotation array index (one old one new).  I think it would be best to put an explicit range check like this:
> 
> -  if (md == NULL)
> +  if (md == NULL || index() >= md->length())
>     return NULL;
>   return md->at(index());
> 
> There are two reasons:  First, although it is likely this is a needless check, the code which ensures length equality for field arrays and annotation arrays is buried in another file (classFileParser) and hard to check.  Second, recent changes to classFileParser actually decouple the field array length from the value declared in the class file, by injecting extra fields into certain system classes.  Although it is (again) unlikely that this will cause a problem, putting a local range check in the code above guarantees an adequate level of safety.
> 
> The "at" method includes an assertion check, which is also good, but assertions are (a) not enabled in product mode and (b) can crash the VM if they fail.
> 
> — John
> 
> On Dec 18, 2012, at 9:05 AM, Joel Borggrén-Franck wrote:
> 
>> New webrev up:
>> 
>> http://cr.openjdk.java.net/~jfranck/8004823/webrev.v7/
>> 
>> - I had to rename the field name for the type annotation byte[] in Field, Constructor and Method.
>> 
>> Also fixed:
>> 
>> - I fixed the mapfile that got reindented.
>> - Added back a comment that I accidentally removed.
>