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

Mono.Cecil does not set the HasFieldRVA field on a FieldDefinition when the InitialData property is set to non-null #728

Closed
jkoritzinsky opened this issue Mar 12, 2021 · 5 comments · Fixed by #733

Comments

@jkoritzinsky
Copy link
Contributor

I'm trying to use the FieldDefinition.InitialData property to emit a static array into an image using the .data declaration feature in ECMA-335.

However, it seems that any data assigned to this field is dropped and not emitted as a .data declaration in the image.

I know that Mono.Cecil supports round-tripping the InitialData field from a loaded assembly back to disk if it hasn't been modified, but it seems that modifying it or assigning it on a new field is unsupported.

@jbevain
Copy link
Owner

jbevain commented Mar 13, 2021

Hi @jkoritzinsky,

Thank you for filing this. That definitely sounds like a bug, which might be annoying because from a quick look that should not happen:

https://github.com/jbevain/cecil/blob/master/Mono.Cecil/FieldDefinition.cs#L114
https://github.com/jbevain/cecil/blob/master/Mono.Cecil/AssemblyWriter.cs#L1595

Any chance you could start a PR with a failing test?

@jkoritzinsky
Copy link
Contributor Author

I'll work on putting together a failing unit test for this.

@jkoritzinsky
Copy link
Contributor Author

While putting together a unit test, I found the issue.

When a new field is created in Mono.Cecil and an array is assigned to the InitialValue field, Mono.Cecil correctly emits the value and will read it back in. However, Mono.Cecil does not set the HasFieldRVA flag, so other tools like ILDasm and ILSpy do not correctly recognize that an RVA has been assigned.

When I manually set the HasFieldRVA flag, the other tools recognize the RVA. So thankfully I have a workaround.

It would be nice if the FieldDefinition class would add that flag when the InitialData property is set. I'll update the issue title.

@jkoritzinsky jkoritzinsky changed the title Mono.Cecil does not support writing out new data assigned to FieldDefinition.InitialData to the final image Mono.Cecil does not set the HasFieldRVA field on a FieldDefinition when the InitialData property is set to non-null Mar 15, 2021
@jbevain
Copy link
Owner

jbevain commented Mar 15, 2021

Hah! That makes sense. Yes we should do that, should be has simple as:

		public byte [] InitialValue {
			// get
			set {
				initial_value = value;
				rva = 0;
				HasFieldRVA = !value.IsNullOrEmpty();
			}
		}

@jkoritzinsky want to send a PR with the test & the fix?

@jkoritzinsky
Copy link
Contributor Author

Yes, I'll send a PR with a targeted test and fix.

jkoritzinsky added a commit to jkoritzinsky/cecil that referenced this issue Mar 15, 2021
jbevain added a commit that referenced this issue Mar 16, 2021
* Implement automatic handling of the HasFieldRVA field attribute.

Fixes #728

* Add FieldDefinition.HasFieldRVA

Co-authored-by: Jb Evain <jb@evain.net>
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 a pull request may close this issue.

2 participants