On Fri, 11 Sep 2026 14:05:39 GMT, Ashutosh Mehra <[email protected]> wrote:
>> src/hotspot/share/code/aotCodeCache.cpp line 4080:
>>
>>> 4078: // Required by initial stubs
>>> 4079:
>>> ADD_EXTERNAL_ADDRESS(SharedRuntime::exception_handler_for_return_address);
>>> // used by forward_exception
>>> 4080: ADD_EXTERNAL_ADDRESS(CompressedKlassPointers::base_addr());
>>
>> Here’s an overall question/comment about the sequencing of
>> `ADD_EXTERNAL_ADDRESS`: It looks like the index assigned to each added addr
>> depend senstively on configuration information, both static build options
>> and even command line options. What prevents the AP and PR (assembly phase
>> and production run) from disagreeing on which addr is at which index? (Or
>> would any such disagreement be benign – which I doubt.) Suppose we had a
>> bug where there was an undetected config drift in the PR which affected the
>> sequencing of the addrs; how would such a bug be detected? Crashes?
>>
>> If we could use some more "belt and suspenders" redundancy here, I have a
>> suggestion: Make the length (`_extrs_length`) be part of the AOT cache
>> config, to be matched between AP and PR. Sample the length at the end of
>> startup, and maybe a few other places (since startup has predictable
>> phases). Make sure the expected length, observed in the AP, shows up in the
>> PR.
>
> In current code I found this code which makes address table dependent on the
> VM config:
>
>
> if (ValueTypeReturnedAsFields) {
>
>
>
> ADD_EXTERNAL_ADDRESS(SharedRuntime::store_value_type_fields_to_buf);
>
>
>
> }
>
>
> So we already have this problem in the current code base.
One partial solution: Fold the conditional flag expression into the macro call,
and have the macro somehow incorporate logic to check or assert that the
condition is the same for AP (as recorded in cache) and TR.
To be clear about priorities here: I don't think there is a significant bug
here. I would support separate config-hardening PRs after this PR. I predict we
are not going to get all the config checks shaken out in one go.
But, a check of extrs-length will catch bugs (or a few at different phases)
will make a broad fence against config alignment bugs on the addrs array. So I
recommend that for this PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r3991003855
