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

Fody Caseless corrupts GitVersionCore on OSX #891

Closed
eatdrinksleepcode opened this issue Jun 4, 2016 · 8 comments
Closed

Fody Caseless corrupts GitVersionCore on OSX #891

eatdrinksleepcode opened this issue Jun 4, 2016 · 8 comments

Comments

@eatdrinksleepcode
Copy link
Contributor

Package building is currently disabled on non-Windows builds. If you enable it, ILRepack in GitVersionExe fails; but after tracing back through ILRepack and Mono.Cecil, I discovered that the actual problem is that GitVersionCore has invalid IL which is produced by the Fody Caseless plugin. Eliminating the Caseless plugin from FodyWeavers.xml in GitVersionCore avoids the corruption and allows ILRepack to complete successfully.

As an example of the corruption, this is the IL generated by mcs for the method ""...

.locals init (
        bool    V_0)
IL_0000:  ldarg.1
IL_0001:  ldarg.0
IL_0002:  ldfld string GitVersion.VersionAssemblyInfoResources.AssemblyVersionInfoTemplates/'<AssemblyVersionInfoTemplates>c__AnonStorey0'::enclosingNamespace
IL_0007:  dup
IL_0008:  brtrue IL_0013

IL_000d:  pop
IL_000e:  ldsfld string [mscorlib]System.String::Empty
IL_0013:  callvirt instance bool string::StartsWith(string)
IL_0018:  stloc.0
IL_0019:  br IL_001e

IL_001e:  ldloc.0
IL_001f:  ret

...and here is the same method after Fody Caseless has modified it:

.locals init (
        bool    V_0)
IL_0000:  ldarg.1
IL_0001:  ldarg.0
IL_0002:  ldfld string GitVersion.VersionAssemblyInfoResources.AssemblyVersionInfoTemplates/'<AssemblyVersionInfoTemplates>c__AnonStorey0'::enclosingNamespace
IL_0007:  dup
IL_0008:  brtrue.s IL_0013

IL_000a:  pop
IL_000b:  ldsfld string [mscorlib]System.String::Empty
IL_0010:  ldc.i4.5
IL_0011:  callvirt instance bool string::StartsWith(string, valuetype [mscorlib]System.StringComparison)
IL_0016:  stloc.0
IL_0017:  br.s IL_0019

IL_0019:  ldloc.0
IL_001a:  ret

Note that in both the unmodified and modified versions, instruction IL_0008 is a conditional jump to instruction IL_0013 (with the modified version using the short form of the instruction). However, unlike in the unmodified version, the modified version has no instruction IL_0013; the jump should instead be to IL_0010.

Fody Caseless does not seem to produce the same corruption on Windows...

Unmodified:

.locals init ([0] bool CS$1$0000)
IL_0000:  ldarg.1
IL_0001:  ldarg.0
IL_0002:  ldfld      string GitVersion.VersionAssemblyInfoResources.AssemblyVersionInfoTemplates/'<>c__DisplayClass7'::enclosingNamespace
IL_0007:  dup
IL_0008:  brtrue.s   IL_0010
IL_000a:  pop
IL_000b:  ldsfld     string [mscorlib]System.String::Empty
IL_0010:  callvirt   instance bool [mscorlib]System.String::StartsWith(string)
IL_0015:  stloc.0
IL_0016:  br.s       IL_0018
IL_0018:  ldloc.0
IL_0019:  ret

Modified:

.locals init ([0] bool CS$1$0000)
IL_0000:  ldarg.1
IL_0001:  ldarg.0
IL_0002:  ldfld      string GitVersion.VersionAssemblyInfoResources.AssemblyVersionInfoTemplates/'<>c__DisplayClass7'::enclosingNamespace
IL_0007:  dup
IL_0008:  brtrue.s   IL_0010
IL_000a:  pop
IL_000b:  ldsfld     string [mscorlib]System.String::Empty
IL_0010:  ldc.i4.5
IL_0011:  callvirt   instance bool [mscorlib]System.String::StartsWith(string,
                                                                       valuetype [mscorlib]System.StringComparison)
IL_0016:  stloc.0
IL_0017:  br.s       IL_0019
IL_0019:  ldloc.0
IL_001a:  ret

Note that csc generated the short form of brtrue to begin with, and Fody did not modify it; whereas mcs generated the long form, and Fody changed it to the short form, but did not correctly adjust the target.

Since disabling Fody Caseless is presumably not an option, as that could have a potentially significant impact on GitVersion functionality, I will look into fixing Fody to correctly handle this case.

@gep13
Copy link
Member

gep13 commented Jun 5, 2016

Any chance @SimonCropp can provide some input to this one? Thanks!

@asbjornu
Copy link
Member

asbjornu commented Jun 6, 2016

@eatdrinksleepcode Would issues like this be easier to discover if Fody ran on Travis (Linux)? If so, I'd love to help setting that up.

@eatdrinksleepcode
Copy link
Contributor Author

@asbjornu Certainly they would :) I had to make some changes to Caseless to get it working on Mono/non-Windows; I will submit a PR with those in the next day or so.

@eatdrinksleepcode
Copy link
Contributor Author

I have a repro and a couple of options for a fix for the corruption issue (and an explanation of why it's not happening on Windows, which was the part that was really bothering me). It will take me a little while to write up the issue in Caseless, but when I do I will link it here.

@asbjornu
Copy link
Member

asbjornu commented Jun 6, 2016

@eatdrinksleepcode Awesome work, thanks!

@JakeGinnivan
Copy link
Contributor

Looks like we just need to update to Caseless 1.4.2 to fix this issue

@eatdrinksleepcode
Copy link
Contributor Author

That was included as part of #890, so this should be resolved.

On Jul 10, 2016, at 1:15 AM, Jake Ginnivan notifications@github.com wrote:

Looks like we just need to update to Caseless 1.4.2 to fix this issue


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@JakeGinnivan
Copy link
Contributor

Awesome

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

4 participants