Fix nested annotation element encoding in StubGenerator (#724) - #725
Open
wasabii wants to merge 1 commit into
Open
Fix nested annotation element encoding in StubGenerator (#724)#725wasabii wants to merge 1 commit into
wasabii wants to merge 1 commit into
Conversation
The object-overload of EncodeElementValue iterated a nested annotation's name/value pairs with i++ instead of i += 2. After the first pair, i lands on a value slot, so (string)v[i] throws InvalidCastException when the value is an array (nested annotation, enum, or array member). This was a porting typo introduced in 3dad0f9 when StubGen was rewritten on top of IKVM.ByteCode; the pre-rewrite loop and the two sibling loops (EncodeAnnotation and the CustomAttributeTypedArgument overload) all use i += 2 correctly. Fixes #724
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #724.
The bug
StubGenerator.EncodeElementValue(theobjectoverload) encodes a nested annotation (TAG_ANNOTATION) whose payload is laid out as name/value pairs:[tag, typeName, name0, value0, name1, value1, …]. The loop advanced by one instead of two:After the first pair,
ilands on a value slot, and(string)v[i]throwsInvalidCastException: Unable to cast object of type 'System.Object[]' to type 'System.String'whenever that value is itself an array (a nested annotation, enum, or array member). It only fires for nested annotations with more than one element, which is why it went unnoticed for so long.The fix
One character —
i++→i += 2— restoring parity with the two sibling loops that were ported correctly (EncodeAnnotationand theCustomAttributeTypedArgumentoverload) and with the pre-rewrite implementation.History
Regression introduced in 3dad0f9 (Aug 18 2024), the rewrite of StubGen onto IKVM.ByteCode. The old
WriteAnnotationElementValueusedi += 2; the rewrite produced three copies of the pattern and two of the three kept the increment. This one didn't.