[lworld] RFR: 8378099: [lworld] Wrong adapter choice when is_interp_only_mode is set [v2]

[Marc Chevalier]

> 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 without the fix, and no crashes with the fix...

Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:

  Copyright

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

Changes:
  - all: https://git.openjdk.org/valhalla/pull/2124/files
  - new: https://git.openjdk.org/valhalla/pull/2124/files/57cc9115..146d4372

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2124&range=01
 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2124&range=00-01

  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/valhalla/pull/2124.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2124/head:pull/2124

PR: https://git.openjdk.org/valhalla/pull/2124