RFR: 8357060: [premain] assert(left >= right) failed: avoid underflow [v2]

[Andrew Dinn]

On Sat, 17 May 2025 02:16:23 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>> This PR fixes a few things in the premain:
>> 1. When storing metadata in aot code cache, `AOTCacheAccess::delta_from_shared_address_base` was incorrectly using `SharedBaseAddress` for computing the offset. It should be using `MetaspaceShared::requested_base_address` because we convert the input address to the requested address.
>> 2. Fixing the above issue results in crash in C1 and C2 compiled code during production run because CompressedKlassPointer::base() value was hardcoded in the generated code. In mainline we emit relocation for `CompressedKlassPointer::base()`. This patch adds the same changes to premain. In addition to that, it also modifies `MacroAssembler::decode_and_move_klass_not_null` and `MacroAssembler::encode_and_move_klass_not_null` which are used by C2 compiled code.
>> 3. Fixing 2 reveals another problem when preload the code. `AOTCodeEntry::_method` can be invalid if the AOT Cache gets mapped to different address than the "requested" address, and can result in crash when accessing `AOTCodeEntry::_method` during preload. Fix is to store the offset of the `AOTCodeEntry::_method` and use the offset on load to get the correct Method pointer.
>> 4. While working on this issue, I realized archived `AOTCodeCache::compile_nmethod` is using archived nmethod to print the assembly. This results in crash as archived nmethod has some state cleaned up. Updated `AOTCodeCache::compile_nmethod` to fix this issue.
>
> Ashutosh Mehra has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix compile failure in zero variant
>    
>    Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>
>  - Add methods in AOTCacheAccess to convert offset to appropriate metadata pointer
>    
>    Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

Looks good apart from maybe clarifying the accessor methods.

src/hotspot/share/cds/aotCacheAccess.hpp line 51:

> 49:   static bool can_generate_aot_code(InstanceKlass* ik) NOT_CDS_RETURN_(false);
> 50: 
> 51:   static uint delta_from_base_address(address addr);

Could we have a comment here to distinguish the operation of `delta_from_base_address` and `convert_offset_to_klass/method.

As I understand it:

`convert_offset_to_method/klass` are used during a production run to retrieve a pointer to a `Method/Klass` located in a loaded archive. The offset argument identifies a delta from the archive's currently mapped shared base address to the start of the object. It will normally have been obtained by reading a value embedded in some other archive entry.

`delta_from_base_address` are used during an assembly run to compute the offset at which a metadata object will be found in the mapped region of an archive under construction. The input argument is the address of a metadata object (`Method/Klass`) loaded by the assembly JVM. Computation of the offset requires mapping the supplied metadata object to a clean copy which resides in the current mapped archive range, remapping the address of that copy to its target location in some requested address range and subtracting that address from the requested base address (the latter being the address at which the shared archive is expected to be remapped). n.b. the offset of the clean copy in the current shared address range may not be the same as the offset of the object in the requested address range.

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

Marked as reviewed by adinn (Committer).

PR Review: https://git.openjdk.org/leyden/pull/68#pullrequestreview-2850307907
PR Review Comment: https://git.openjdk.org/leyden/pull/68#discussion_r2095422323