Skip to content

[mono] ILStrip sorts custom attribute table - #87923

Merged
steveisok merged 4 commits into
dotnet:mainfrom
jandupej:ilstrip-sort
Jun 22, 2023
Merged

[mono] ILStrip sorts custom attribute table#87923
steveisok merged 4 commits into
dotnet:mainfrom
jandupej:ilstrip-sort

Conversation

@jandupej

Copy link
Copy Markdown
Contributor

This prevents custom attribute table corruption by sorting it as the last step when stripping an assembly. Addresses #85414.

The PR will have to be backported to net7.0.

@ghostghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 22, 2023
@ghostghost assigned jandupejJun 22, 2023
@jandupejjandupej 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 22, 2023
@jandupejjandupej added this to the 8.0.0 milestone Jun 22, 2023
}
}

void SortCustomAttributes()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you please add a comment about why we need to sort the custom attributes here?

@steveisok

Copy link
Copy Markdown
Member

/backport to release/7.0-staging

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/5349160200

{
CustomAttributeRow row_left = (CustomAttributeRow)left;
CustomAttributeRow row_right = (CustomAttributeRow)right;
return row_left.Parent.RID.CompareTo(row_right.Parent.RID);

@lambdageeklambdageekJul 5, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is subtly wrong. the problem is that RID on a cecil metadata token masks out the token type.

We actually have to reconstruct the custom attribute coded-index.
something like:

varleftParentCodedIdx=Utilities.CompressMetadataToken(CodedIndex.HasCustomAttribute,row_left.Parent);varrightParentCodedIdx=Utilities.CompressMetadataToken(CodedIndex.HasCustomAttribute,row_right.Parent);returnleftParentCodedIdx.CompareTo(rightParentCodedIdx);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This fixed the issue I was running into on #88167

akoeplinger added a commit to akoeplinger/runtime that referenced this pull request Jul 5, 2023
The change in dotnet#87923 was subtly wrong, the problem is that RID on a Cecil metadata token masks out the token type.
We actually have to reconstruct the custom attribute coded-index.
github-actionsBot pushed a commit that referenced this pull request Jul 5, 2023
The change in #87923 was subtly wrong, the problem is that RID on a Cecil metadata token masks out the token type.
We actually have to reconstruct the custom attribute coded-index.
lewing pushed a commit that referenced this pull request Jul 5, 2023
The change in #87923 was subtly wrong, the problem is that RID on a Cecil metadata token masks out the token type.
We actually have to reconstruct the custom attribute coded-index.
@ghostghost locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jandupej@steveisok@lambdageek@akoeplinger