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

Fix "DLL Characteristics" meanings in Optional Header #2147

Merged
merged 1 commit into from
Sep 6, 2020

Conversation

srutzky
Copy link
Contributor

@srutzky srutzky commented Sep 4, 2020

Made changes to several of the "Meaning" values of the "DLL Characteristics" flags in order to match the official documentation ( https://docs.microsoft.com/en-us/dotnet/api/system.reflection.portableexecutable.dllcharacteristics ):

  1. Added definitions for first four values (0x01, 0x02, 0x04, and 0x08); it seems that they are "Reserved" instead of "Unused". I'm not sure if these values will ever be seen, but I figured it best to include the definitions since they seems to exist.
  2. Changed wording in option 0x1000 to be "must" instead of "should" as it's both clearer and matches the documentation.
  3. For option 0x20, the description here was "ASLR with 64-bit address space", while the documentation has it as "The image can handle a high entropy 64-bit virtual address space". After reviewing https://en.wikipedia.org/wiki/Address_space_layout_randomization I believe it's best to favor the documentation but also include 'ASLR' since that's really what the option pertains to.

Please note that options 0x80 and 0x4000 (copied below) are, for some odd reason, missing from the official documentation. I don't see a reason to remove them as the documentation is not 100% correct and sometimes things are left out. Should we add a comment in the C# code on each of those two lines indicating the disparity? For now I'm not doing that but might not be a bad idea to make it visible there since nobody will ever see this note here:

<0080> Code integrity checks are enforced
<4000> Image supports Control Flow Guard

 
Take care,
Solomon...
https://SqlQuantumLeap.com/

Made changes to several of the "Meaning" values of the "DLL Characteristics" flags in order to match the official documentation ( https://docs.microsoft.com/en-us/dotnet/api/system.reflection.portableexecutable.dllcharacteristics ):

1. Added definitions for first four values (0x01, 0x02, 0x04, and 0x08); it seems that they are "Reserved" instead of "Unused". I'm not sure if these values will ever be seen, but I figured it best to include the definitions since they seems to exist.
2. Changed wording in option 0x1000 to be "must" instead of "should" as it's both clearer and matches the documentation.
3. For option 0x20, the description here was "ASLR with 64-bit address space", while the documentation has it as "The image can handle a high entropy 64-bit virtual address space". After reviewing https://en.wikipedia.org/wiki/Address_space_layout_randomization I believe it's best to favor the documentation but also include 'ASLR' since that's really what the option pertains to.

Please note that options 0x80 and 0x4000 (copied below) are, for some odd reason, missing from the official documentation. I don't see a reason to remove them as the documentation is not 100% correct and sometimes things are left out. Should we add a comment in the C# code on each of those two lines indicating the disparity? For now I'm not doing that but might not be a bad idea to make it visible there since nobody will ever see this note here:

<0080> Code integrity checks are enforced
<4000> Image supports Control Flow Guard
@siegfriedpammer
Copy link
Member

The flags you mention are described in the native PE documentation: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#dll-characteristics

Not sure, which one is more accurate. I guess these flags are left out because the have no use in managed binaries?

@dgrunwald dgrunwald merged commit db11c6d into icsharpcode:master Sep 6, 2020
@srutzky
Copy link
Contributor Author

srutzky commented Sep 14, 2020

@siegfriedpammer and @dgrunwald : I see that this has already been merged, but my plan had been to post a question to the documentation team asking for clarification on these items. Is there any value in me still doing that? Please let me know.

Take care,
Solomon..

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.

3 participants