Skip to content
/ cecil Public
forked from jbevain/cecil
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

FieldRVA alignment mono/cecil tree #33

Closed

Conversation

davidwrighton
Copy link
Member

Port of PR jbevain#817 to mono/cecil tree

In support of dotnet/runtime#60948 the linker (an assembly rewriter) will need to be able to preserve the alignment of RVA based fields which are to be used to create the data for CreateSpan records

This is implemented by adding a concept that RVA fields detect their required alignment by examining the PackingSize of the type of the field (if the field type is defined locally in the module)

This has been approved in the jbevain tree, but we need this sooner rather than later, so I've made a direct PR here as well.

davidwrighton and others added 3 commits January 18, 2022 14:46
In support of dotnet/runtime#60948 the linker (an assembly rewriter) will need to be able to preserve the alignment of RVA based fields which are to be used to create the data for `CreateSpan<T>` records

This is implemented by adding a concept that RVA fields detect their required alignment by examining the PackingSize of the type of the field (if the field type is defined locally in the module)
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
@davidwrighton
Copy link
Member Author

@vitek-karas Can you approve/merge this PR?

@vitek-karas
Copy link
Member

I can't merge - maybe @agocke or @marek-safar can?

@agocke
Copy link
Member

agocke commented Jan 19, 2022

No luck on my side

@marek-safar
Copy link

It seems the change is about to get in upstream. Unless there is some delay I'd prefer to pull from upstream instead of cherry-picking changes to make the synchronization easier in the future (I'm happy to do that it's just one git command).

@davidwrighton
Copy link
Member Author

@marek-safar I agree. Upstream now has the fix in it. It looks like it is time to take a pull from upstream though. Could do that?

Thanks.

@marek-safar
Copy link

@davidwrighton done

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.

4 participants