[lworld] RFR: 8361082: [lworld] RewriteBytecodesInlineTest fails with SIGSEGV [v3]

[Coleen Phillimore]

On Wed, 2 Jul 2025 18:21:07 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> `RewriteBytecodesInlineTest` fails after # due to a new log message printing the pending exception name even though there may not be a pending exception:
>> `PENDING_EXCEPTION->klass()->name()->as_C_string()`
>> 
>> This patch refactors the loadable descriptor handling used in `SystemDictionary::load_shared_class` to better illustrate how the loadable descriptors property is handled and it corrects the log messages to be consistent with the messages used in the class file parser. Verified with tier 1-5 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Cleanup
>  - Multilined another log message
>  - Changed long log messages to multi-line strings

I have a few comments - maybe some of it can be refactored and shared? Also thank you for reformatting the logging lines.

src/hotspot/share/classfile/systemDictionary.cpp line 1078:

> 1076: }
> 1077: 
> 1078: // Pre-load class referred to in non-static null-free instance field. These fields trigger MANDATORY loading

Add period at the end of the sentence.

src/hotspot/share/classfile/systemDictionary.cpp line 1109:

> 1107: 
> 1108:   assert(real_k != nullptr, "Sanity check");
> 1109:   if (real_k->access_flags().is_identity_class()) {

It looks like you could refactor the check for identity and abstract in to InstanceKlass::check_inline_field(TRAPS) and share the code with ClassFileParser. (?)

src/hotspot/share/classfile/systemDictionary.cpp line 1195:

> 1193: 
> 1194:       if (fs.is_null_free_inline_type()) {
> 1195:         bool check = preload_from_null_free_field(ik, class_loader, sig, field_index, CHECK_NULL);

This is a bit confusing that it both has a check for return and CHECK_NULL.  Add a comment that some preloading might fail but not fatally for the shared archive if the class changed in the app.

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

PR Review: https://git.openjdk.org/valhalla/pull/1498#pullrequestreview-2980341411
PR Review Comment: https://git.openjdk.org/valhalla/pull/1498#discussion_r2180795881
PR Review Comment: https://git.openjdk.org/valhalla/pull/1498#discussion_r2180801339
PR Review Comment: https://git.openjdk.org/valhalla/pull/1498#discussion_r2180804792