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

Coleen Phillimore coleen.phillimore at oracle.com
Tue Sep 6 19:29:38 UTC 2016


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



More information about the hotspot-runtime-dev mailing list