Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow nop guards for interface call sites #16122

Merged
merged 5 commits into from Oct 19, 2022

Conversation

jdmpapin
Copy link
Contributor

...unless inliner has already proved that the receiver is an instance of a particular class that implements the expected interface. (However, it appears that it is currently difficult or maybe impossible to get such a bound despite tryToRefineReceiverClassBasedOnResolvedTypeArgInfo()).

A passing vgnop-based interface guard can guarantee the receiver type is as expected by the inlined body, but only if we already know before the guard that the receiver implements the expected interface. Here, an interface type bound is insufficient to know that, because bytecode verification does not guarantee any particular receiver type at invokeinterface.

As such, in the absence of a class (i.e. non-interface) type bound on the receiver, all targets must use profiled guard. Additionally, the profiled guard must ensure on the hot side that the receiver is an instance of the interface expected at this call site.

There was one circumstance where it was possible to generate a profiled guard with method test for a method defined by a class that does not implement the interface. Inliner now uses a VFT test instead in this situation. Only hot compilations performed during startup are affected.

It was also possible to generate an interface (nop) guard for a single implementer method defined by a class that does not implement the interface. Since this must now be a profiled guard, inliner will only accept single implementers defined by classes that do implement the interface. Others will be rejected in the hope that it will be possible to generate a VFT test based on profiling.

Without these precautions, it would be possible for the JIT to inline a single implementing method with a nop guard, and for execution to flow into the inlined body with a receiver that is not an instance of the class expected by the inlined code. The inlined code could then access fields of the receiver as though it were of the expected type.


This series has a few additional changes mainly to reduce the performance impact of all of the additional profiled guards:

  • Correct interface call site assertion
  • Heuristically consider VFT test for interface call profiled guards
  • Be more confident about single interface implementer profiled guards
  • Base interface inlining determination directly on VM startup phase

...unless inliner has already proved that the receiver is an instance of
a particular class that implements the expected interface. (However, it
appears that it is currently difficult or maybe impossible to get such a
bound despite tryToRefineReceiverClassBasedOnResolvedTypeArgInfo()).

A passing vgnop-based interface guard can guarantee the receiver type is
as expected by the inlined body, but only if we already know before the
guard that the receiver implements the expected interface. Here, an
interface type bound is insufficient to know that, because bytecode
verification does not guarantee any particular receiver type at
invokeinterface.

As such, in the absence of a class (i.e. non-interface) type bound on
the receiver, all targets must use profiled guard. Additionally, the
profiled guard must ensure on the hot side that the receiver is an
instance of the interface expected at this call site.

There was one circumstance where it was possible to generate a profiled
guard with method test for a method defined by a class that does not
implement the interface. Inliner now uses a VFT test instead in this
situation. Only hot compilations performed during startup are affected.

It was also possible to generate an interface (nop) guard for a single
implementer method defined by a class that does not implement the
interface. Since this must now be a profiled guard, inliner will only
accept single implementers defined by classes that do implement the
interface. Others will be rejected in the hope that it will be possible
to generate a VFT test based on profiling.

Without these precautions, it would be possible for the JIT to inline a
single implementing method with a nop guard, and for execution to flow
into the inlined body with a receiver that is not an instance of the
class expected by the inlined code. The inlined code could then access
fields of the receiver as though it were of the expected type.
Apparently the class pointer in the profiled guard comes directly from
the call site object, not from its virtual guard selection.
In particular, for the profiled guards generated instead of nop guards
due to safety requirements in the case of a single implementer.

VFT test excludes some receivers that could run the inlined method body,
but it's a cheaper test at runtime. It's now preferred:

- when the inlined method is defined by a final class,

- when the inlined method is defined by a class that has not been
  extended (yet),

- when profiling indicates that only a single receiver type occurs.
These guards used to be nop guards. They no longer are, but only because
certain kinds of bytecode are technically allowed to be loaded. Bytecode
that is problematic in this way should be exceedingly rare though, so
treat these guards more like the nop guards we used to have by marking
the taken side cold and by making sure to allow privatized argument
rematerialization.
The JIT can re-enter STARTUP_STATE after leaving it, and it can remain
in that state for much longer than one might naively expect, inhibiting
these heuristics.
@pshipton
Copy link
Member

jenkins test sanity alinux64,aix,xmac,amac,win,zlinux jdk11,jdk17 depends eclipse/omr#6773

@pshipton pshipton merged commit 0d7572f into eclipse-openj9:master Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants