RFR: 8372617: Save and restore stubgen stubs when using an AOT code cache

[Andrew Dinn]

On Fri, 21 Nov 2025 00:06:22 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> This PR adds save and restore of all generated stubs to the AOT code cache on x86 and aarch64. Other arches are modified to deal with the related generic PAI changes.
>> 
>> Small changes were required to the aarch64 and x86_64 generator code in order to meet two key constraints:
>> 1. the first declared entry of every stub starts at the first instruction in the stub code range
>> 2. all data/code cross-references from one stub to another target a declared stub entry
>
> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 611:
> 
>> 609:       return start;
>> 610:     }
>> 611: 
> 
> This code pattern repeats a lot. Can we move it into `load_archive_data()`?
> 
> address start = load_archive_data(stub_id);
> if (start != nullptr) {
>   return start;
> }
> 
> 
> And have an other specialized `load_archive_data` if you need `end` value.

Yes, I think I can probably refactor this to make it simpler and avoid returning end. I'll push a revised version soon. Return of `end` is mostly a hangover from an older version but it does still serve a purpose (see below). Originally, it was not clear what all the other specialized cases were going to be but I thinkit is now fairly clear that we only need to cater for:

 - multiple entries other than the start address
 - multiple extra addresses
 - both the above.

n.b. entries and extras are distinguished by one simple criterion: the former can be accessed from AOT code while the latter cannot. So, entries need to be registered with the AOT address table beneath `load_entry` while extras can be omitted (although they almost certainly need to be registered with the current runtime by the caller). That distinction ought to be written down in a comment somewhere, also the acceptable values for both:

`start` clearly must be non-null. Additional entries may be `nullptr` or in the range `[start, end)`. Extra addresses may be `nullptr` or in the range `[start, end]` i.e. extras can address the first byte after the end of the stub.

This is needed so that an extra can be used to mark the (exclusive) end address of a range that covers the tail of a stub. So far, it has always been the case that extra addresses occur in triples identifying UnsafeMemoryAccess handler regions, `(open, close, handler)`. I'm leaving it open as to whether any other uses might turn up which is why processing of extras happens in the caller rather than  under `load_entry`.

`load_entry` now knows and verifies how many entries it ought to be retrieving and accepts an `extras` array if and only if there are extra addresses in the archive. It also range checks the entry/extra addresses. So, all a caller needs to do is assign extra entries to the outputs passed by the caller then validate the extras count and process them appropriate to whatever the stub understands them to mean.

`end` was originally being returned by `load_entry` to allow the caller to range check both entry and extra addresses but that check is now redundant. It is still needed because of an unnecessary complication in the current code. Addresses are converted to offsets before saving in the archive and translated back to addresses when loading. `store_entry` does ensure that all entry or extra offsets addresses lie within the range `[start, end)` and `[start, end]` respectively. However, `nullptr` and `end` both get saved to the archive as `start - end`. That means both of them get restored as `nullptr`. Clients are expected to know whether a restored `nullptr` at some offset in the extras array is really an `end` address and substitute `end` in its place (e.g. in the case where `close == nullptr` really means `close == end`). Because of that `load_entry` has to make `end` visible to the caller. If I change things so that `nullptr` is saved as UINT_MAX in that will avoid the ambiguity, all
 owing the client to use the value without an extra check.

@vnkozlov ^^

> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 13115:
> 
>> 13113:   StubRoutines::aarch64::init_AOTAddressTable(external_addresses);
>> 13114:   AOTCodeCache::publish_external_addresses(external_addresses);
>> 13115: }
> 
> This and `*init_AOTAddressTable()` methods should be under `#if INCLUDE_CDS`

Done

> src/hotspot/share/runtime/stubRoutines.cpp line 317:
> 
>> 315: // Non-generated init method
>> 316: 
>> 317: void StubRoutines::init_AOTAddressTable() {
> 
> Can this body be in platform specific file.

Yes, I'll add an implementation to file stubRoutines_xxx.cpp for each platform.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28433#discussion_r2556329608
PR Review Comment: https://git.openjdk.org/jdk/pull/28433#discussion_r2556346740
PR Review Comment: https://git.openjdk.org/jdk/pull/28433#discussion_r2568620542
PR Review Comment: https://git.openjdk.org/jdk/pull/28433#discussion_r2556361072