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

Add modreq and modopt to imported FieldRefs #921

Closed
wants to merge 1 commit into from

Conversation

Tejes
Copy link

@Tejes Tejes commented Aug 2, 2023

When importing a FieldReference using the DefaultReflectionImporter, the volatile modifier was lost. This resulted in a
System.MissingFieldException at runtime if one wanted to access such field via Cecil emitted IL. The bare type was used for the field, excluding the modreq IsVolatile modifier.

This commit wraps the field's type into RequiredModifierType and OptionalModifierType objects, this way the writer emits correct IL for the volatile field reference.

I hope my commit message is clear in what the problem was, and what my solution is. It definitely needs a test, which I am unsure of where to put. I only found one method calling ModuleDefinition.ImportReference(FieldInfo) and that deals with generics.
I've tested my code with a static volatile bool[] field in a C# library. I don't know any other modreq at the moment, and I think modopt is only used for C++. I am also not sure how multiple modreq and modopt should be handled (or where do I even encounter them), in what order.

I am new to Cecil, so my code might be janky a bit, but I'm open to suggestions. I didn't want to just open an issue and wait for someone to fix it, I've tried, and it works for me, treat it as an MVC.

When importing a FieldReference using the DefaultReflectionImporter, the
volatile modifier was lost. This resulted in a
System.MissingFieldException at runtime if one wanted to access such
field via Cecil emitted IL. The bare type was used for the field,
excluding the modreq IsVolatile modifier.

This commit wraps the field's type into RequiredModifierType and
OptionalModifierType objects, this way the writer emits correct IL for
the volatile field reference.
@jbevain
Copy link
Owner

jbevain commented May 21, 2024

Thank you for your PR! I've adjusted it in #943.

@jbevain jbevain closed this May 21, 2024
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.

2 participants