RFR 8166358: Re-enable String verification in java_lang_String::create_from_str()

[David Holmes]

Hi Coleen,

Overall this looks good. A few minor comments below.

On 22/05/2020 6:34 am, coleen.phillimore at oracle.com wrote:
> Summary: Check for invalid strings in class names in debug mode, and 
> only verify valid strings in create_for_str().
> 
> See bug for more information on why this commented out code needs to be 
> conditional but not commented out.  Tested with mach5 tier1-6, and new 
> test for new -Xcheck:jni check.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/2020/8166358.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8166358

src/hotspot/share/classfile/javaClasses.cpp

I'm a bit confused about adding the validity check at the end of 
java_lang_String::create_from_str. Don't we need to ensure it is valid 
before doing those other Utf8 operations on it?

     // This check is too strict because the input string is not 
necessarily valid UTF8.
     // For example, it may be created with arbitrary content via 
jni_NewStringUTF.
!   if (UTF8::is_legal_utf8((const unsigned char*)utf8_str, 
(int)strlen(utf8_str), false)) {

The comment needs adjusting now that you are guarding the check with 
is_legal_utf8().

---

src/hotspot/share/classfile/systemDictionary.cpp

+      return NULL;

Indentation appears off by one.

---

src/hotspot/share/prims/jvm.cpp

I think this comment block:

// Since exceptions can be thrown, class initialization can take place
// if name is NULL no check for class name in .class stream has to be made.

should be read as:

// Since exceptions can be thrown, class initialization can take place.
// If name is NULL no check for class name in .class stream has to be made.

so your changes to the comment:

// Since exceptions can be thrown, class initialization can take place
// if name is NULL. The class name is checked in the .class stream.

don't appear appropriate.

Thanks,
David
-----

> Thanks,
> Coleen