On Wed, 16 Sep 2026 21:25:36 GMT, Vladimir Kozlov <[email protected]> wrote:
>> Thinking more about a future failure mode – there may be future edits to
>> stubs that don’t synch with config tests. If a zero ever comes from a query
>> to the stubs table, will an assertion catch it before it is used in code?
>>
>> Partial answer, looking at `AOTCodeAddressTable::address_for_id`: The zero
>> pops out with no assert or fatal error. Other error conditions are caught
>> with `fatal`. It is not clear whether zero is allowed in this API, although
>> `(address)-1` is a sentinel value checked for by callers, so it seems
>> unlikely that a second sentinel can be there.
>>
>> Suggest either a debug-only assert that checks for zero (followed by
>> returning the real sentinel) or better a straight `fatal` call.
>>
>>
>> @@ -2395,15 +2395,21 @@ address AOTCodeAddressTable::address_for_id(int idx)
>> {
>> return nullptr;
>> }
>> uint id = (uint)idx;
>> + address result = nullptr;
>> // no need to compare unsigned id against 0
>> if (id < _extrs_length) {
>> - return _extrs_addr[id - _extrs_base];
>> + result = _extrs_addr[id - _extrs_base];
>> }
>> + else //reflow me
>> if (id >= _stubs_base && id < _c_str_base) {
>> - return _stubs_addr[id - _stubs_base];
>> + result = _stubs_addr[id - _stubs_base];
>> }
>> + else //reflow me
>> if (id >= _c_str_base && id < (uint)(_c_str_base + _C_strings_count)) {
>> - return address_for_C_string(id - _c_str_base);
>> + result = address_for_C_string(id - _c_str_base);
>> }
>> + if (result != nullptr) {
>> + return result; // could be sentinel (address)-1 in some cases
>> + }
>> fatal("Incorrect id %d for AOT Code Cache addresses table", id);
>> return nullptr;
>
> @rose00 I are looking on old code in `laden:premain2` branch? That code is
> outdated.
>
> Current code in this PR already does something similar:
> [aotCodeCache.cpp#L4920](https://github.com/vnkozlov/jdk/blob/6765f00d850c29e0f05d480d78df6383bef70e9e/src/hotspot/share/code/aotCodeCache.cpp#L4920)
>
> The only missing check is check for null the `result` as you suggested.
Yes, I was looking at the wrong file, so the diff was wrong in detail, but you
got my point; thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30778#discussion_r4051611754
