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

Place fatal asserts in FE queries that JITServer should not call #17355

Merged
merged 1 commit into from May 25, 2023

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented May 8, 2023

The following FE queries:

TR_J9VMBase::getReferenceFieldAt(uintptr_t objectPointer, uintptr_t fieldOffset)
TR_J9VMBase::getReferenceFieldAtAddress(uintptr_t fieldAddress)
TR_J9VMBase::getVolatileReferenceFieldAt(uintptr_t objectPointer, uintptr_t fieldOffset)
TR_J9VMBase::getStaticReferenceFieldAtAddress(uintptr_t fieldAddress) 

are not called by JITServer because they look inside objects and require VM access to prevent objects from being moved by GC.
This commit places fatal asserts in the FE queries mentioned above, to make it clear that such queries cannot (and should not) be called by JITServer.

The following FE queries:
TR_J9VMBase::getReferenceFieldAt(uintptr_t objectPointer, uintptr_t fieldOffset)
TR_J9VMBase::getReferenceFieldAtAddress(uintptr_t fieldAddress)
TR_J9VMBase::getVolatileReferenceFieldAt(uintptr_t objectPointer, uintptr_t fieldOffset)
TR_J9VMBase::getStaticReferenceFieldAtAddress(uintptr_t fieldAddress)
are not called by JITServer because they look inside objects and require VM access
to prevent objects from being moved by GC.
This commit places fatal asserts in the FE queries mentioned above, to make it clear
that such queries cannot (and should not) be called by JITServer.

Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label May 8, 2023
@mpirvu mpirvu added this to In progress in JIT as a Service via automation May 8, 2023
@mpirvu
Copy link
Contributor Author

mpirvu commented May 8, 2023

jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit jdk20

@mpirvu mpirvu requested a review from jdmpapin May 8, 2023 19:02
@mpirvu
Copy link
Contributor Author

mpirvu commented May 8, 2023

@jdmpapin When you have time, could you please review/merge this PR? Thanks

@jdmpapin
Copy link
Contributor

Thanks, @mpirvu!

I guess this set is sufficient because the other problematic queries take an object address as an argument, and we can't possibly pass an object address to one of those until after we've obtained one, and the first one to be obtained could only have come from one of these disallowed queries.

@mpirvu
Copy link
Contributor Author

mpirvu commented May 25, 2023

@jdmpapin gentle reminder to review/merge this PR. Thanks

@jdmpapin
Copy link
Contributor

Oh sorry, I think I thought that I had asked you to confirm my understanding above, but apparently I hadn't. Regardless, I think I understood properly, and you don't seem to have wanted to clarify anything, so I'll go ahead

@jdmpapin
Copy link
Contributor

jdmpapin commented May 25, 2023

Changes are JITServer-specific, and JITServer builds have passed

@jdmpapin jdmpapin merged commit 4bb727b into eclipse-openj9:master May 25, 2023
9 checks passed
JIT as a Service automation moved this from In progress to Done May 25, 2023
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
Development

Successfully merging this pull request may close these issues.

None yet

2 participants