-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix LibraryImportGenerator crash with malformed CollectionMarshaller and ElementMarshaller #117467
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
Conversation
Co-authored-by: agocke <[email protected]>
Co-authored-by: agocke <[email protected]>
// Bug: https://github.com/dotnet/runtime/issues/117448 | ||
// This test case produces expected diagnostics instead of crashing | ||
// TODO: Enable with proper expected diagnostics in follow-up work | ||
// yield return new object[] { ID(), CodeSnippets.ImproperCollectionWithMarshalUsingOnElements, new DiagnosticResult[] { /* expected diagnostics */ } }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line needs to be uncommented to actually run the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! The test is now uncommented and includes the proper expected diagnostics. All tests pass. (commit 5b2cc0e)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try reverting your fix and verify that the test failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified! I reverted the fix and confirmed the test failed with a NullReferenceException
crash in the LibraryImportGenerator at line 145 in MarshallingAttributeInfo.cs. Then restored the fix and confirmed the test passes with expected diagnostics. The test correctly catches the bug when the fix is not present.
Co-authored-by: agocke <[email protected]>
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
The LibraryImportGenerator would crash with a
NullReferenceException
when processing malformed collection marshallers with element marshallers. This occurred when:CollectionElementMarshallingInfo
was null but code attempted to access itsElementDependencies
propertyManagedType
propertyChanges Made
Fixed two null reference exceptions:
In
MarshallingAttributeInfo.cs
line 145: Added null check forCollectionElementMarshallingInfo
before accessingElementDependencies
In
GeneratorDiagnostic.cs
line 41: Added null check forManagedType
before accessingDiagnosticFormattedName
with fallback to"<unknown>"
Test Case
Added the test case from PR #117447 that reproduces the issue. The test case demonstrates a malformed collection marshaller where:
MyList<T>
but methods acceptNotMyList
parametersElementIndirectionDepth = 1
The generator now produces proper diagnostic errors instead of crashing:
SYSLIB1052
: Invalid ElementIndirectionDepth configurationSYSLIB1051
: Unsupported type with proper error handlingVerification
Fixes #117448.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.