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

Account for Truncated Signatures in a JITServerHelpers Assertion #20883

Merged

Conversation

luke-li-2003
Copy link
Contributor

Add additional checks so that the assertion "(memcmp(dst, str, J9UTF8_TOTAL_SIZE(str)) == 0)" in packCallBack() also accounts for the dst signature being truncated for generated classes. Prevents the non-fatal assertion from failing.

Fixes #20046

Add additional checks so that the
assertion "(memcmp(dst, str, J9UTF8_TOTAL_SIZE(str)) == 0)"
in packCallBack() also accounts for the dst signature being
truncated for generated classes. Prevents the non-fatal
assertion from failing.

Fixes eclipse-openj9#20046

Signed-off-by: Luke Li <[email protected]>
@luke-li-2003 luke-li-2003 requested a review from dsouzai as a code owner January 3, 2025 20:26
@luke-li-2003
Copy link
Contributor Author

@mpirvu can you have a look at this?

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.

Looks ok to me.
What guarantees do we have that the memcmp will not access memory beyond the UTF-8 boundaries? The name _generatedPrefixLength suggests that we only compare a few characters in the beginning, but is there a guarantee that the length of src and dst is at least as large as _generatedPrefixLength?
@cjjdespres As the original author, could also please review?

@mpirvu mpirvu self-assigned this Jan 3, 2025
@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Jan 3, 2025
@luke-li-2003
Copy link
Contributor Author

The lengths should match because dst should be created by truncating str to the length of _generatedPrefixLength. We can also add another assertion beforehand to check it explicitly (like str->length >= _generatedPrefixLength && dst->length == _generatedPrefixLength)

@mpirvu
Copy link
Contributor

mpirvu commented Jan 4, 2025

jenkins compile xlinuxjit jdk21

@mpirvu mpirvu merged commit 8b21775 into eclipse-openj9:master Jan 4, 2025
4 checks passed
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.

Non-fatal assert triggered in sanity.functional with JITServer and AOT cache during ROM class packing
2 participants