[lworld] RFR: 8378099: [lworld] Wrong adapter choice when is_interp_only_mode is set [v2]
Tobias Hartmann
thartmann at openjdk.org
Wed Feb 18 13:25:02 UTC 2026
On Wed, 18 Feb 2026 13:05:32 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:
>> In `SharedRuntime::get_resolved_entry` when `current->is_interp_only_mode()` we need to go to the interpreter, and thus we must always return a c2i adapter (and not the entry point of another compiled method).
>>
>> https://github.com/openjdk/valhalla/blob/d5e3620d5a0b13cce9c19bafd5b2dd881b8bf9e9/src/hotspot/share/runtime/sharedRuntime.cpp#L1635-L1641
>>
>> Yet, here, we disregard the kind of call, or the caller. Yet, under, we make the correct distinction:
>>
>> https://github.com/openjdk/valhalla/blob/d5e3620d5a0b13cce9c19bafd5b2dd881b8bf9e9/src/hotspot/share/runtime/sharedRuntime.cpp#L1643-L1652
>>
>> Indeed, if the caller is C1, the call won't be scalarized, so we don't need the packing logic that would be required for a scalarized call from C2, for instance... Overall, we must distinguish the 3 same cases as when `is_interp_only_mode` is not set. The only difference is that instead of picking the compiled entry point if available, and only an adapter as a fallback, we must take the adapter. Right now, the code is correct only if `!caller_does_not_scalarize && (is_static_call || is_optimized)` (so, especially not calls from C1).
>>
>> I've slightly changed the shape of the code to make that more clear, and also to be less likely to forget a case if we add new variations in the calling convention!
>>
>> If this error doesn't show up so often, that's because `is_interp_only_mode()` is only set by JVMTI:
>>
>> https://github.com/openjdk/valhalla/blob/d5e3620d5a0b13cce9c19bafd5b2dd881b8bf9e9/src/hotspot/share/prims/jvmtiThreadState.cpp#L322-L328
>> and
>> https://github.com/openjdk/valhalla/blob/d5e3620d5a0b13cce9c19bafd5b2dd881b8bf9e9/src/hotspot/share/prims/jvmtiThreadState.inline.hpp#L151-L153
>>
>> So, without JVMTI, it is impossible to observe this bug, which is unfortunate. So, how to test that, not to do the same error again? A test involving JVMTI would be brittle, and we can do better than adding yet another test: making better usage of existing tests. Now, `StressCallingConvention` will sometimes make `get_resolved_entry` behaves as if `is_interp_only_mode` is set. Doing so causes SO MANY crashes in `TestCallingConventionC1`, which is expected. With the fix, it works again as expected.
>>
>> The frequency of `is_interp_only_mode` being randomly true when stressing is currently set at a very arbitrary 1/2^10 which is around 0.1%. I've also tested with a probability of 1/2, and I get basically the same behavior: plenty of crashes with the improved stress wi...
>
> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>
> Copyright
Marked as reviewed by thartmann (Committer).
-------------
PR Review: https://git.openjdk.org/valhalla/pull/2124#pullrequestreview-3819889475
More information about the valhalla-dev
mailing list