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 string equals if other is null and string constant pool #3

Open
wants to merge 1 commit into
base: declarative-config
Choose a base branch
from

Conversation

mmuesly
Copy link
Member

@mmuesly mmuesly commented Oct 4, 2023

The loop from internedString to host and back to guest crashes the constant handling in the constant pool. I think, it should be sufficient to just hand over the internedString to SPouT and add it to the pool. String equals has not gracefully handled comparison with null in all cases.

This PR is recommended as replacement to #2. I personally prefer the loop over the underlying VM wherever possible instead of faithfully modeling the string library behavior in the substitution methods.

@mmuesly mmuesly requested a review from fhowar October 4, 2023 15:56
The loop from internedString to host and back to guest crashes the constant handling in the constant pool.
I think, it should be sufficient to just hand over the internedString to SPouT and add it to the pool.
String equals has not gracefully handled comparison with null in all cases
@@ -2136,11 +2136,9 @@ private void putPoolConstant(VirtualFrame frame, int top, char cpi, int opcode)
} else if (constant instanceof StringConstant) {
assert opcode == LDC || opcode == LDC_W;
StaticObject internedString = pool.resolvedStringAt(cpi);
Meta meta = getMeta();
StaticObject obj = meta.toGuestString(meta.toHostString(internedString));
Copy link
Member Author

Choose a reason for hiding this comment

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

@fhowar I don't know why you added this and whether it is really necessary.
The constant op code does not need it and all other exceptions should be handled in the markObjectWithIfTaint method, IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant