RFR: 8327156: Avoid copying in StringTable::intern(oop, TRAPS) [v6]

David Holmes dholmes at openjdk.org
Wed Nov 13 05:38:46 UTC 2024


On Tue, 12 Nov 2024 15:31:11 GMT, Casper Norrbin <cnorrbin at openjdk.org> wrote:

>> src/hotspot/share/classfile/javaClasses.cpp line 393:
>> 
>>> 391: #ifdef ASSERT
>>> 392:   // This check is too strict on older classfile versions
>>> 393:   if (UTF8::is_legal_utf8((const unsigned char*)utf8_str, utf8_len, /*version_leq_47*/false))
>> 
>> I don't see the point of this check. Any validation would/should be done when the Symbol is created, not when it is later used.
>
> The problem is that the string could technically be correct according to its classfile version, but could then fail the checks done here. The older classfile versions had relaxed requirements, and these don't play well the checks here. 
> 
> So when the Symbol is created the string is not technically "wrong", it's just that some functions (like here) assume that the string is correct according to newer definitions of "correct". It is not certain that the checks of these strings would succeed, we can only guarantee that on newer "correct" UTF8.

Okay I see why now. I had previously failed to determine where the `version_leq_47` special handling had come from and exactly what it meant. But I think I have now tracked it down to [JDK-4324508](https://bugs.openjdk.org/browse/JDK-4324508). I'm not 100% convinced that we are using `is_legal_utf8` correctly everywhere (in terms of what we pass for `version_leq_47`), or even that we generally handle such old utf8 "strings" correctly, but that is a separate issue.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21325#discussion_r1839442334


More information about the hotspot-dev mailing list