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

Fix JITServer thunk pointer handling issue #16684

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

cjjdespres
Copy link
Contributor

The client thunk pointer should be returned from setJ2IThunk at the JITServer, not the (heap-allocated) server thunk pointer.

Signed-off-by: Christian Despres despresc@ibm.com

Attn @mpirvu. This fixes an issue that came up while testing #16650.

The client thunk pointer should be returned from setJ2IThunk at the
JITServer, not the (heap-allocated) server thunk pointer.

Signed-off-by: Christian Despres <despresc@ibm.com>
@mpirvu mpirvu self-assigned this Feb 8, 2023
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Feb 8, 2023
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me and it resembles the non-jitserver code where setJ2IThunk() returns the location in the SCC where the thunk was stored.
However, I would like to understand why this change is essential.

@mpirvu
Copy link
Contributor

mpirvu commented Feb 8, 2023

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit jdk17

@mpirvu mpirvu merged commit 3d709fa into eclipse-openj9:master Feb 9, 2023
@mpirvu
Copy link
Contributor

mpirvu commented Feb 10, 2023

I was interested in the effects of this PR, or to be more exact, how come JITServer didn't experience crashes without this fix.
This is a summary of my findings:

J2I thunk pointers are used in snippets. The codegen emits thunk pointers for unresolved interface/non-interface calls and for resolved interface calls under AOT. This is decided in TR::X86PicDataSnippet::shouldEmitJ2IThunkPointer().
The encoding of the J2I thunk pointer is done with encodeJ2IThunkPointer().
The presence of a thunk pointer can be checked with TR::X86PicDataSnippet::_hasJ2IThunkInPicData.
The snippet code also generates relocation records of type TR_J2IVirtualThunkPointer. When this is processed during an AOT load (or at a JVM client), TR_RelocationRecordThunks::applyRelocation() is called, which calls TR_RelocationRecordThunks::relocateAndRegisterThunk(). This one calls a static relocateAndRegisterThunk() (which searches to see if the thunk with given signature already exists, and if not, it searches the SCC, allocates code memory, copies the thunk code from SCC into code cache and returns thunk address in outThunkAddress) and then calls relocateJ2IVirtualThunkPointer() to patch the J2I thunk pointer with the value found locally.

JITServer non-AOT scenario:

  • server does not find thunk address at the client, allocates a thunk using heap memory and calls setJ2IThunk
  • setJ2IThunk sends a message to client which registers the thunk code with the VM and returns this address. However, in the code prior to this PR, setJ2IThunk returns the address local to the server
  • server embeds the local (heap allocated) thunk pointer into the method body (in the snippet)
  • server sends the compiled body to client
  • client performs relocations: based on thunk signature it finds the thunk address registered with VM and patches the code with it. Everything works just fine.

JITServer AOT scenario:

  • server does not find thunk address at the client, allocates a thunk using heap memory and calls setJ2IThunk
  • setJ2IThunk sends a message to client which stores the thunk code in SCC and returns this address. However, in the code prior to this PR, setJ2IThunk returns the address local to the server
  • server embeds the local (heap allocated) thunk pointer into the method body (in the snippet)
  • server sends the compiled body to client
  • client performs relocations: based on thunk signature it finds the thunk address in SCC, registers that address with VM and patches the code with it.
  • Everything should work ok, but if SVM is used there is also a validation record and svm->validateJ2IThunkFromMethodRecord(thunkID, thunkAddress) could fail the AOT load.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants