RFR: 8321509: False positive in get_trampoline fast path causes crash [v3]

[Vladimir Kozlov]

On Tue, 2 Jul 2024 11:19:57 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   cleanup
>
> This solution looks ok to me as far as jdk mainline is concerned. However, I think there is a problem as far Leyden is concerned.
> 
> The code changes Relocation::pd_call_destination to always expect its associated call to be embedded within an nmethod when orig_addr is null (i.e. when it is called with no args as reloc.pd_call_destination()). This is where the problem arises.
> 
> Currently, Leyden calls Relocation::pd_call_destination() from CallRelocation::destination() (and also from trampoline_stub_Relocation::destination()) when storing an nmethod to the CDS code cache. It needs to do this in order to be able to track relocs of type virtual_call_type, opt_virtual_call_type, static_call_type and runtime_call_type (also trampoline_stub_type). That is because all these relocs need their call destination to be adjusted when the nmethod is restored from the CDS code cache.
> 
> However, we already have prototype code in Leyden to store generated blobs to the CDS code cache. These blobs may legitimately include runtime_call_type relocs which also need tracking and adjusting at restore. For example, shared runtime or compiler stubs may call out to the JVM. Likewise, stubs in a stub generator blob may need to call out to the JVM or to a stub in some earlier generated blob. So, Leyden will need to call CallRelocation::destination() in cases where the associated call is embedded in a non-nmethod. Note that these calls will never employ trampolines.
> 
> The obvious fix is to modify Relocation::pd_call_destination so that it drops through to call MacroAssembler::pd_call_destination if the incoming blob is not an nmethod.

@adinn is right. I thought that it mostly affect code during codeBlob expansion but it is not.
I applied patch to Leyden/premain repo and hit assert when we generate AOT code because we still processing CodeBuffer before nmethod is created:

#  Internal Error (/work/leyden/open/src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp:59), pid=15382, tid=28419
#  assert(cb != nullptr && cb->is_nmethod()) failed: nmethod expected
#
# JRE version: Java(TM) SE Runtime Environment (24.0) (fastdebug build 24-internal-2024-06-26-1746082.vkozlov...)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 24-internal-2024-06-26-1746082.vkozlov..., mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, bsd-aarch64)
...
Current CompileTask:
C1:  F9322 C0 Q0 S6270 2840    b    2       java.util.zip.InflaterInputStream::close (34 bytes)

Stack: [0x000000016cdec000,0x000000016cfef000],  sp=0x000000016cfed1d0,  free space=2052k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.dylib+0x11dd564]  VMError::report_and_die(int, char const*, char const*, char*, Thread*, unsigned char*, void*, void*, char const*, int, unsigned long)+0x544  (nativeInst_aarch64.cpp:59)
V  [libjvm.dylib+0x11ddd14]  VMError::report_and_die(Thread*, unsigned int, unsigned char*, void*, void*)+0x0
V  [libjvm.dylib+0x58f220]  print_error_for_unit_test(char const*, char const*, char*)+0x0
V  [libjvm.dylib+0xe3ddb4]  NativeCallTrampolineStub::destination(nmethod*) const+0x0
V  [libjvm.dylib+0x12ac4]  SCCache::write_relocations(CodeBuffer*, unsigned int&)+0x34c

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

PR Comment: https://git.openjdk.org/jdk/pull/19796#issuecomment-2204337485