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

Minor discrepancy between SetOffset() and WriteTo() #63

Closed
mazegen opened this issue May 25, 2016 · 5 comments
Closed

Minor discrepancy between SetOffset() and WriteTo() #63

mazegen opened this issue May 25, 2016 · 5 comments

Comments

@mazegen
Copy link

mazegen commented May 25, 2016

In class MetaDataHeader, SetOffset() uses actual offset when aligning strings but WriteTo() aligns the strings using the string length instead of the actual offset. This can lead to exception being thrown from VerifyWriteTo() when the MetaDataHeader items change and computing the alignment differs between SetOffset() and WriteTo().

@0xd4d
Copy link
Collaborator

0xd4d commented May 25, 2016

Are you changing heap.Name between calls to SetOffset() and WriteTo()? That will break the code since obviously SetOffset() must know the name in order to calculate its size.

if not, do you have a sample that triggers this condition?

@mazegen
Copy link
Author

mazegen commented May 25, 2016

For example, if the reserved Flags value (ECMA-335 name) is not written, SetOffset expects 108 bytes to be written but only 106 bytes are actually written to the output although the code changes are valid:

SetOffset(...) {
....
//length += 4;   // Flags and Streams
length += 2;   // Streams only
...

;

WriteTo(...) {
...
//writer.Write((byte)(options.StorageFlags ?? 0));
//writer.Write(options.Reserved2 ?? 0);

I understand that almost noone modifies the header this way and I expect this behaviour to be resolved as "Won't Fix"; I just wanted to let you know.

@0xd4d
Copy link
Collaborator

0xd4d commented May 25, 2016

You've modified the writer code so instead of writing StorageFlags,Reserved2,heaps.Count, you now only write heaps.Count? How does the CLR know this and correctly parses the new header without the 2 bytes before the heap count field? Is there a new flag I don't know about or forgot?

@mazegen
Copy link
Author

mazegen commented May 25, 2016

The CLR can't parse it, I work on my own runtime. I should have mentioned it before.

@0xd4d
Copy link
Collaborator

0xd4d commented May 25, 2016

This header will always be 4-byte aligned so there's only a problem if you change the size of the header so the heaps aren't 4-byte aligned, as you did in your modified code. I'll close this as a Won't-Fix 😎 but it should be trivial for you to fix it for your custom header.

@0xd4d 0xd4d closed this as completed May 25, 2016
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

No branches or pull requests

1 participant