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

PEverify issues using 2.10.0+ NuGet versions of OpenXml #734

Closed
Szaamaan opened this issue Jun 3, 2020 · 7 comments · Fixed by #744
Closed

PEverify issues using 2.10.0+ NuGet versions of OpenXml #734

Szaamaan opened this issue Jun 3, 2020 · 7 comments · Fixed by #744

Comments

@Szaamaan
Copy link

Szaamaan commented Jun 3, 2020

Description

OpenXML NuGet libraries with version 2.10.0 and upwards fail PEverify.

Information

  • DocumentFormat.OpenXml Version: 2.10.0 and upwards
  • Microsoft (R) .NET Framework PE Verifier. Version 4.0.30319.0

Repro

Run PEverify (available in Developer Command Prompt for VS, I'm using 2019 version) on DocumentFormat.OpenXML.dll

Observed

peverify DocumentFormat.OpenXml.dll

Microsoft (R) .NET Framework PE Verifier.  Version  4.0.30319.0
Copyright (c) Microsoft Corporation.  All rights reserved.

[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1[T]::get_Length][offset 0x00000001] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1[T]::GetIndex][offset 0x00000044] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ValidatorCollection::GetEnumerator][offset 0x00000001] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1+Enumerator[T]::get_Current][offset 0x00000001] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1+Enumerator[T]::MoveNext][offset 0x00000012] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1+PropertyEntry[T]::get_Property][offset 0x00000001] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1+PropertyEntry[T]::get_Value][offset 0x00000001] Cannot change initonly field outside its .ctor.
[IL]: Error: [C:\Project\bin\Debug\DocumentFormat.OpenXml.dll : DocumentFormat.OpenXml.Framework.ElementPropertyCollection`1+PropertyEntry[T]::SetValue][offset 0x00000001] Cannot change initonly field outside its .ctor.
8 Error(s) Verifying DocumentFormat.OpenXml.dll

While the library works fine in simple console applications / libraries, I found it will fail to work when ILmerged into MS CRM Sandbox (which is running on partial trust towards external DLLs), which is what led me to check the DLL using PEverify. Previous version (2.9.1 and earlier) don't have PEverify errors and their usage in MS CRM Sandbox doesn't cause issues.

Expected

PEverify should not show any errors. DLL should be usable after ILmerge in MS CRM Sandbox (partial trust env).

@twsouthwick
Copy link
Member

@Szaamaan thanks for the issue. This is due to using readonly structs internally. Appears there was an issue discussing this dotnet/roslyn#22485.

It appears this can cause issues in partial trust so apparently it's a breaking change. What framework version are you running? I'll add the compat flag to .NET Framework builds (partial trust doesn't exist on Core so standard versions don't need it)

@Szaamaan
Copy link
Author

Szaamaan commented Jul 8, 2020

I was using .NET 4.6.2 (I think; might have been 4.5.2). As per the description, I was having issues actually getting the DLL to run in MS CRM Sandbox after using ILMerge to make a single library (common way of getting external libraries when creating MS CRM plugins), so I'm not 100% clear if the specific framework version I was using matters or if it's more of an issue with ILmerge not working fine with these PEverify issues.

@twsouthwick
Copy link
Member

Yes, it matters, as we also support .NET Core, etc and just wanted to make sure this is only on .NET Framework. I have a fix for this to ensure compat with PEVerify on framework builds.

@twsouthwick
Copy link
Member

According to the Roslyn bug, this can be hit in partial trust. ILMerge is kind of best effort and can cause it's own bugs that we're probably not in a place to fully test. If this is due to ILMerge, we'd need to look down a different path. However, the errors you're seeing are also visible without IL merge, so once the merge is in, you can grab the CI build and give it a try.

@twsouthwick
Copy link
Member

twsouthwick commented Jul 8, 2020

@Szaamaan Can you verify that your process works with the latest builds that have peverify compat enabled? Use the CI feed described in the README, and you'll need at least v2.11.1-ci0007

@twsouthwick twsouthwick reopened this Jul 8, 2020
@Szaamaan
Copy link
Author

Szaamaan commented Jul 9, 2020

@twsouthwick The CI version you linked works fine in an MS CRM Sandbox (partial trust) after ILmerging. 😄 I honestly don't think that ILmerge had anything to do with this process (I was merely trying to be thorough in my description of the issue); it's far more likely that partial-trust was the trigger for the issue I had. After all, the DLLs did work for me in CRM after ILmerging in a non-sandbox (full-trust) setting... but while great for debugging, it's not a good method to deploy online CRM solutions. 😉

@twsouthwick
Copy link
Member

Perfect! Yeah, this issue is a known one for partial trust apparently. If we had the ACPTA attribute in the assembly it would have compiled fine, but thankfully there is a compat switch for peverify. We'll get that released to NuGet soon. Thanks for verifying!

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