On Thu, 26 Mar 2026 10:36:22 GMT, Damon Fenacci <[email protected]> wrote:

>> # Issue
>> Many AppCDS test asserts with `assert(false) failed: Address 
>> 0x00007f0dfc2923e0 for <unknown>/('verify_oop: r11: broken oop oop_result, 
>> "broken oop in call_VM_base" 
>> (src/hotspot/cpu/x86/macroAssembler_x86.cpp:1353)') is missing in AOT Code 
>> Cache addresses table` when run with `-Xcomp --enable-preview`
>> 
>> # Cause
>> The crash happens during AOT cache dumping seemingly because 
>> `-XX:+VerifyOops` causes the adapter to use addresses that the AOT doesn't 
>> know about. In particular, `verify_oop`/`verify_oop_addr` add a message 
>> C‑string and reference the verify‑oop stub entry. 
>> `AOTCodeCache::write_relocations()` tries to serialize those relocations, 
>> `AOTCodeAddressTable::id_for_address()` can’t resolve them and crashes.
>> In this case the issue happens with `--enable-preview` because it creates an 
>> adapter for scalarized arguments and, after creating the oop from the 
>> arguments, `get_vm_result_oop` invokes `verify_oop_msg`. Without 
>> `--enable-preview` the path is never taken and the missing "registration" 
>> isn’t exercised (I fear that this crash could potentially be triggered by 
>> some other (non preview) code but the fix doesn't need to distinguish 
>> between preview/non-preview).
>> 
>> # Fix
>> Apparently we need to make the two addresses used by `verify_oop_msg` 
>> "visible" to AOT: register the debug message string and add 
>> `StubRoutines::verify_oop_subroutine_entry_address()` to the AOT “extrs” 
>> table.
>> 
>> # Testing
>> Tier 1-3+
>> Failing CDS tests before and after
>
> Damon Fenacci has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains nine additional 
> commits since the last revision:
> 
>  - Merge branch 'lworld' into JDK-8379913
>  - JDK-8379913: fix typo
>  - JDK-8379913: disable adapters caching if VerifyOops
>  - JDK-8379913: revert adding cache entries
>  - JDK-8379913: remove include
>  - JDK-8379913: add VerifyOops condition
>  - JDK-8379913: remove useless comment
>  - JDK-8379913: aarch64
>  - JDK-8379913: [lworld] AppCDS test asserts with -Xcomp --enable-preview

That looks like a reasonable fix to me but it's only needed when preview is 
enabled, right?

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

Marked as reviewed by thartmann (Committer).

PR Review: 
https://git.openjdk.org/valhalla/pull/2233#pullrequestreview-4015117774

Reply via email to