gluck / il-repack

Open-source alternative to ILMerge
Apache License 2.0
1.16k stars 214 forks source link

Repacking Harmony in Debug mode fails #355

Closed pardeike closed 5 months ago

pardeike commented 5 months ago

Hi,

my open source project Harmony uses this task to pack its "Fat" releases. It works fine but we discovered that the "DebugFat" release does not work on .NET 5 or newer. Packing itself isn't the problem because ReleaseFat works just fine.

The problem is very easy to replicate by cloning the project (master) and running tests for DebugFat and ReleaseFat.

There is an active discussion going on on my discord server where the guys from the MonoMod.Core project (a dependency Harmony has) discuss that this is related to them using some function pointer here which matches the exception I get when running/testing DebugFat:

System.TypeLoadException : Could not load type 'CORINFO_METHOD_INFO' from assembly 'System.Runtime.Serialization.Formatters, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.

which is also randomly giving other assemblies (so not related to System.Runtime.Serialization.Formatters at all).

Quote from @nike4613 in the discord:

If you want to discuss this issue, you are welcome to temporarily join my discord server. The channel is called harmony-contributors.

Reference to original post: https://github.com/ravibpatel/ILRepack.Lib.MSBuild.Task/issues/49#issuecomment-2019474516

KirillOsenkov commented 5 months ago

Hmm I just cloned and built Harmony and I get an error: image

Have you seen this?

Happy to take a look at this as well.

KirillOsenkov commented 5 months ago

I'm guessing this is because I don't have .NET 5.0 installed so it can't resolve against .NET 5.0 (and 3.1?)

pardeike commented 5 months ago

Could very well be. Harmony support a wide range of .net versions so I guess you need the sdks

KirillOsenkov commented 5 months ago

OK here's the fix for the build to not require all the .NET Core SDKs installed: https://github.com/pardeike/Harmony/pull/604

KirillOsenkov commented 5 months ago

OK indeed I can confirm something is wrong.

dotnet tool update -g checkbinarycompat
cd C:\Harmony\Harmony\bin\DebugFat\net8.0
checkbinarycompat 0harmony.dll

produces BinaryCompatReport.txt:

In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve assembly reference to 'Microsoft.Win32.Primitives, Version=8.0.0.0, PublicKeyToken=b03f5f7f11d50a3a'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve type reference 'MonoMod.Core.Interop.CoreCLR' in assembly 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve type reference 'MonoMod.Core.Interop.CoreCLR/V21' in assembly 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve type reference 'MonoMod.Core.Interop.CoreCLR/V21/CORINFO_METHOD_INFO' in assembly 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve type reference 'MonoMod.Core.Interop.CoreCLR/V70' in assembly 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': Failed to resolve type reference 'MonoMod.Core.Interop.CoreCLR/V70/AllocMemArgs' in assembly 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
In assembly '0Harmony, Version=2.3.3.0, PublicKeyToken=null': reference 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51' resolved from 'C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.3\System.Text.Json.dll' as 'System.Text.Json, Version=8.0.0.0, PublicKeyToken=cc7b13ffcd2ddd51'
KirillOsenkov commented 5 months ago

checkbinarycompat is a tool I wrote to resolve all assembly refs, type refs and member refs and ensure they are present in the referenced assembly: https://www.nuget.org/packages/checkbinarycompat#readme-body-tab

KirillOsenkov commented 5 months ago

Indeed, I'm seeing 10 invalid tokens in the merged assembly:

calli      valuetype MonoMod.Core.Interop.CoreCLR/CorJitResult(native int,native int,native int,valuetype  [ERROR: INVALID TOKEN 0x23000007] MonoMod.Core.Interop.CoreCLR/V21/CORINFO_METHOD_INFO*,uint32,uint8**,uint32*)
KirillOsenkov commented 5 months ago

Easy peasy! Apparently we forgot to rewrite types for parameters of calli instruction. Confirmed checkbinarycompat is happy now.

KirillOsenkov commented 5 months ago

Published https://www.nuget.org/packages/ILRepack/2.0.29 and https://www.nuget.org/packages/ILRepack.Lib/2.0.29

pardeike commented 5 months ago

Wow, cool, thanks for the swift response and fix. Also nice to get a PR from you so it builds regardless of sdk installs.

pardeike commented 5 months ago

Hmm, I still cannot use the resulting DebugFat package with the same error message as before. The change did fix something but it still fails (and ReleaseFat still works).

KirillOsenkov commented 5 months ago

OK I'll look again and maybe this time I'll actually try the fix :)

KirillOsenkov commented 5 months ago

do you have a specific test that fails in debugfat?

KirillOsenkov commented 5 months ago

also to be conpletely sure, can you please git clean -xdf and try again?

pardeike commented 5 months ago

my bad, everything works as expected