RFR: 8312401: SymbolTable::do_add_if_needed hang when requesting length exceeds max_symbol_length [v2]

[David Holmes]

On Sat, 22 Jul 2023 00:39:19 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> Please review the simple fix to resolve infinite loop in SymbolTable::do_add_if_needed caused by extra long symbol string that exceeds Symbol::max_length(). See JDK-8312401 for details.
>> 
>> The jtreg test is converted from a test case constructed by @cushon.
>
> Jiangli Zhou has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update according to David Holmes' comments.
>   - In java_lang_String::as_symbol and java_lang_String::as_symbol_or_null, check input string length and truncate to max symbol length if exceeding.
>   - Change java_lang_Throwable::create_initialization_error to avoid using Symbol for message.

Okay on changing to `char*` for the exception message, but not sure about the rest. Will wait to see what @coleenp has to say. I think we have some dubious/fragile code here.

Thanks.

src/hotspot/share/classfile/javaClasses.cpp line 574:

> 572: // Return a Symbol for the input Java string. A new Symbol is created if one
> 573: // does not exist. If the length of the input string exceeds Symbol::max_length,
> 574: // the string is truncated to Symbol::max_length.

I don't think this is generally the correct way to approach this. We should only be using `String::as_symbol` when we know the String will fit into a Symbol - if it doesn't then we shouldn't be using this process. So any errors here should be fatal IMO. Each of the clients of this API should be examined to check valid usage i.e. it is okay for field/methods names due to 64K limit on those, but not for class/interface names - and certainly not for arbitrary strings like exception messages! Relatedly the usages of `java_lang_Throwable::detail_message` should also be examined.

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

PR Review: https://git.openjdk.org/jdk/pull/14938#pullrequestreview-1542530441
PR Review Comment: https://git.openjdk.org/jdk/pull/14938#discussion_r1271654335