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

[release/7.0-staging] [mono] ILStrip sorts custom attribute table #87933

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jun 22, 2023

Backport of #87923 to release/7.0-staging

/cc @steveisok @jandupej

Customer Impact

When an assembly is trimmed with ILStrip, corruption of the custom attribute table can occur. This can cause searching for type attributes to fail, resulting in (rare) runtime crashes. Such an error has been observed by a customer #85414. The PR restores sorting to assembly's custom attribute table as the last step of ILStrip, resolving the said error.

Testing

Open a trimmed assembly in ILSpy and verify that all expected attributes are in place. The customer-reported example lacked all attributes for System.DateTimeResult in System.Private.CoreLib.dll, while it should have reported IsByRefLikeAttribute, ObsoleteAttribute and CompilerFeatureRequiredAttribute. When fixed ILStrip is used, the expected attributes are present.

Risk

Low. There are no new features. The length of the custom attribute table is not changed, its rows are merely sorted by Parent key.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 22, 2023
@jandupej jandupej added area-Build-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 23, 2023
@steveisok steveisok added the Servicing-consider Issue for next servicing release review label Jun 23, 2023
@jandupej
Copy link
Member

Approved by tactics via email.

@jandupej jandupej added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 27, 2023
@akoeplinger
Copy link
Member

@jandupej you can merge the PR then since we have the new staging branch setup.

@jandupej jandupej merged commit d5ad559 into release/7.0-staging Jun 27, 2023
176 of 181 checks passed
@jandupej jandupej deleted the backport/pr-87923-to-release/7.0-staging branch June 27, 2023 12:36
@ghost ghost locked as resolved and limited conversation to collaborators Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Build-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants