RFR: 8319343: Improve CDS module graph support for --add-modules option [v2]

[Calvin Cheung]

On Wed, 16 Oct 2024 23:18:42 GMT, John R Rose <jrose at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @rose00 comment
>
> src/hotspot/share/classfile/modules.cpp line 697:
> 
>> 695:       st.print("%s%s", prefix, m);
>> 696:       last_string = m;
>> 697:       prefix = ",";
> 
> You use comma as the separator.  Folks will have various opinions, but I think you should use a character which is positively forbidden by the JVMS 4.2.3.  That says, basically, that any character is fine (in the classfile format) EXCEPT control characters.  The most visible control character is probably `'\n'` so I suggest using that instead of comma.  Your key-strings will look like groups of sorted lines, one module name per line.  That's plenty readable.
> 
> If you keep comma, nothing will break, unless some smart guy figures out how to cook up a classfile that has a module name in it with a comma inside, that looks like two other legitimate modules names separated by that comma.  Something odd could happen next.  Better to avoid it from the start by using a truly illegal module name character.
> 
> (If these were class names I'd be suggesting `';'` or `'.'` not `'\n'` because the rules for class names are different, in the JVMS.  Again, none of these issues come up with inputs which are compiled from normal Java sources, but they can appear with craftily crafted classfiles.)
> 
> https://docs.oracle.com/javase/specs/jvms/se23/html/jvms-4.html#jvms-4.2.3

Thanks for taking a look, John.
I've made the change based on your suggestion.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21553#discussion_r1803986162