RFR (M): 8161224: CONSTANT_NameAndType_info permits references to illegal names and descriptors

[Rachel Protacio]

Thanks for the review, Coleen! I'll change it to "method".
Rachel

On 9/6/2016 3:29 PM, Coleen Phillimore wrote:
>
> Rachel,  I have one small comment.
>
> http://cr.openjdk.java.net/~rprotacio/8161224.00/src/share/vm/classfile/classFileParser.cpp.udiff.html 
>
>
> + // Format check Methodref name and signature
>
>
> Can you change this to just "method", or say Methodref, 
> InterfaceMethodref and InvokeDynamic, since it affects more than just 
> Methodref?
>
> Other than this comment, this change looks really good!
>
> Thanks,
> Coleen
>
> On 9/1/16 5:30 PM, Rachel Protacio wrote:
>> Hello!
>>
>> Please review this fix, which addresses a few issues related to 
>> incomplete format checking with NameAndType names and signatures.
>>
>> First, the code that should have been format checking the strings in 
>> later classfile versions was in fact just checking for periods, so 
>> I've rewritten it to call verify_unqualified_name(). Second, the 
>> checks were (depending on the version) only performed when referenced 
>> through Methodref/Fieldref/InterfaceMethodref/InvokeDynamic, meaning 
>> that non-referenced NameAndType bytecodes did not get checked like 
>> they were supposed to. My change enforces the spec in both aspects. 
>> To summarize:
>>
>> The existing code had:
>> - strict checks for pre-5.0
>> - incomplete/non-spec-compliant checks for 5.0-and-later
>> - no checks for un-referenced NameAndType (A) names and (B) 
>> 6.0-and-earlier signatures.
>>
>> My change has:
>> - the same strict checks for pre-5.0
>> - complete/spec-compliant checks for 5.0-and-later
>> - all checks moved to the NameAndType section so all names and 
>> signatures will be checked regardless of whether NameAndType is 
>> referenced.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8161224
>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8161224.00/
>>
>> Testing: The jck tests which had been failing for this bug now pass, 
>> along with all other jck vm tests. Also tested with JPRT and RBT 
>> hotspot_all and noncolo tests.
>>
>> A compatibility request has been approved for this change.
>>
>> Thank you,
>> Rachel
>