RFR: 8343789: Move mutable nmethod data out of CodeCache [v14]

[Dean Long]

On Fri, 25 Apr 2025 12:03:41 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Boris Ulasevich has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>> 
>>  - swap matadata and jvmci data in outputs according to data layout
>>  - cleanup
>>  - returning oops back to nmethods. jtreg: Ok, performance: Ok. todo: cleanup
>>  - Address review comments: cleanup, move fields to avoid padding, fix CodeBlob purge to call os::free, fix nmethod::print, update Layout description
>>  - add a separate adrp_movk function to to support targets located more than 4GB away
>>  - Force the use of movk in combination with adrp and ldr instructions to address scenarios
>>    where os::malloc allocates buffers beyond the typical ±4GB range accessible with adrp
>>  - Fixing TestFindInstMemRecursion test fail with XX:+StressReflectiveCode option:
>>    _relocation_size can exceed 64Kb, in this case _metadata_offset do not fit into int16.
>>    Fix: use _oops_size int16 field to calculate metadata offset
>>  - removing dead code
>>  - a bit of cleanup and addressing review suggestions
>>  - rework movoop for not_supports_instruction_patching case: correcting in ldr_constant and relocations fixup
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/cfab88b1...bc8c590c
>
> src/hotspot/share/code/nmethod.cpp line 1505:
> 
>> 1503:     CHECKED_CAST(_oops_size, uint16_t, align_up(code_buffer->total_oop_size(), oopSize));
>> 1504:     uint16_t metadata_size = (uint16_t)align_up(code_buffer->total_metadata_size(), wordSize);
>> 1505:     JVMCI_ONLY(CHECKED_CAST(_jvmci_data_size, uint16_t, align_up(compiler->is_jvmci() ? jvmci_data->size() : 0, oopSize)));
> 
> This cast is lossy in that `jvmci_data->size()` returns an int. It caused a `double free or corruption (out)` crash in Graal in the case where a `JVMCINMethodData` had a very long name. We've fixed this by [limiting the length of the name](https://github.com/openjdk/jdk/pull/24753) but I'm wondering if there was some special reason for this cast? If so, can you please add extra logic preventing this code from running off the end of allocated memory:
> 
> #if INCLUDE_JVMCI
>     if (compiler->is_jvmci()) {
>       // Initialize the JVMCINMethodData object inlined into nm
>       jvmci_nmethod_data()->copy(jvmci_data);
>     }
> #endif
> 
> If not, please remove the cast.

The cast was added by 8331087, which reduced the supported JVMCI data size to uint16_t.  I don't remember this issue with long names coming up during that review, so I guess we all missed it.  @dougxc please file a bug so we can track this.  It seems like JVMCINMethodData::copy should do something like truncate long names rather than blindly assuming it has enough space.  If uint16_t is unreasonably small for JVMCI nmethod data we could revert that change and make it 32 bits again.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21276#discussion_r2065196967