7

Closed

BadImageFormatException with Units of Measure

description

The following raises System.BadImageFormatException when built using the Debug configuration in Visual Studio 2013 with F# 3.1.1
The platform makes no difference, and neither does toggling "Prefer 32-bit".
However, the code will run successfully in F# Interactive and when built using the Release configuration.
Removing the type annotations from T.H makes the problem go away.
type T() =
    member this.H<[<Measure>]'u> (x : int<'u>) = x

    member this.F<[<Measure>]'u> (x : int<'u>) =
        let g x =
            this.H x

        g x // Raises System.BadImageFormatException

[<Measure>] type M

[<EntryPoint>]
let main argv = 
    let result = T().F (LanguagePrimitives.Int32WithMeasure<M> 0)
    printfn "Result: %A" result
    0

file attachments

Closed Oct 17, 2014 at 8:18 PM by latkin

comments

BillHally wrote Jun 24, 2014 at 11:38 AM

This appears to be the same issue as Nonverifiable code generated with units of measure conversion.

I've just run peverify against it and received the same error message as in that issue:

[IL]: Error: [... : Program+T::F][offset 0x0000000B][found ref 'Microsoft.FSharp.Core.FSharpFunc`2[System.Int32,System.Int32]'][expected ref 'Microsoft.FSharp.Core.FSharpTypeFunc'] Unexpected type on the stack.

marklam wrote Jul 15, 2014 at 3:23 PM

The code generation is incorrect for --debug+ or --debug-
But the code is correct if compiled with --optimize+ (for debug+ or debug-)

With --debug- --optimize-, the peverify fails at offset 0xA:
    IL_000a: callvirt instance object [FSharp.Core]Microsoft.FSharp.Core.FSharpTypeFunc::Specialize<int32>()
    IL_000f: unbox.any class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<int32, int32>
The object it's trying to call FSharpTypeFunc::Speciailize on is declared as
class nested assembly auto ansi serializable beforefieldinit g@15
    extends class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<int32, int32>

marklam wrote Jul 17, 2014 at 9:50 AM

Removed 'In Debug' from the title, because it's actually optimized vs non-optimized.

JohannesRudolph wrote Sep 26, 2014 at 4:06 PM

I second this, just tripped over the same issue. I'd consider this a major problem (you don't want to generate unverifiable IL in any circumstance from a compiler)

latkin wrote Sep 26, 2014 at 5:00 PM

Note that the provided sample code compiles to an unverifiable assembly in all versions of F# since (at least) F# 3.0. It happens to run without crashing when compiled with 3.0, but the codegen is still incorrect. In 3.1.0+ you get BadImageFormatException at runtime.

latkin wrote Sep 26, 2014 at 5:19 PM

Not to be pedantic, but .NET compilers don't want to accidentally generate unverifiable IL. Sometimes unverifiable code is intentional and desirable, e.g. the F# NativePtr module, or the unsafe keyword in C#.

Of course, this is not one of those cases! :-)

JohannesRudolph wrote Sep 26, 2014 at 10:39 PM

I'm a bit flattered that this is a "known bug", and out in the wild for so long. Either only a few people are seriously using Units of Measure or the quality standards/agility tradeofs for f# are just totally different to what I'm used to from c#.

Having saif that, I may give this a shot since I happen to know a thing or two about IL codegen. Any pointers where to start?

KevinRansom wrote Sep 27, 2014 at 12:55 AM

Johannes please do have a go at fixing this. It would be great to emit correct code in this situation. I look forward to applying your PR.

Thanks

Kevin

JohannesRudolph wrote Sep 27, 2014 at 10:49 AM

So I had a stab today, but unfortunately I'm haven't gotten very far.

Relevant places

  • ilxgen.fs contains translates the AST to to an intermediate IL representation (ILX). ILX already does closure transformation
    ** The relevant places in
  • pubclo.fs then "splits and chops" closures so that e.g. for generic closures each FsharpTypeFunc derived closure class adds a single type arguments via the specialize method.

Brain Dump / Log

I think the core of the codegen for closures is here:
https://github.com/fsharp/fsharp/blob/master/src/fsharp/ilxgen.fs

GenClosureTypeDef is called from
  • GenDelegateExpr (that's not it, it uses .Invoke())
  • GenLambdaClosure
  • GenObjectExpr
  • GenSequenceExpr
The original Error from compiling the reported code
Not Verifiable: Incompatible this argument on stack with method signature expected
'Microsoft.FSharp.Core.FSharpTypeFunc' but got 'Microsoft.FSharp.Core.FSharpFunc`2<int, int> (Complex)'
for a call to 'Microsoft.FSharp.Core.FSharpTypeFunc:Specialize<int> ()' at 0x000b

internal class g@10 : FSharpFunc<int, int>
{
    public FSharp.Test.T this;

    internal g@10( FSharp.Test.T this )
    {
        this.this = this;
    }

    public override int Invoke( int x )
    {
        return this.this.H( x );
    }
}

Location:
.method public hidebysig 
    instance int32 F (
        int32 x
    ) cil managed 
{
    // Method begins at RVA 0x2060
    // Code size 28 (0x1c)
    .maxstack 4
    .locals init (
        [0] class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<int32, int32>,
        [1] int32
    )

    IL_0000: ldarg.0
    IL_0001: newobj instance void '<StartupCode$FSharp-Test>.$Test'/g@10::.ctor(class FSharp.Test.T)
    IL_0006: stloc.0
    IL_0007: nop
    IL_0008: ldloc.0
    IL_0009: ldarg.1
    IL_000a: stloc.1
    IL_000b: callvirt instance object [FSharp.Core]Microsoft.FSharp.Core.FSharpTypeFunc::Specialize<int32>()
    IL_0010: unbox.any class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<int32, int32>
    IL_0015: ldloc.1
    IL_0016: callvirt instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<int32, int32>::Invoke(!0)
    IL_001b: ret
} // end of method T::F
Correct would be to
  • either emit a further closure class with a FSharpTypeFunc::Specialize method, which would return the g@10 named class
  • or do not emit a call to .Specialize and omit the following unbox.any and ldlocXX instructions
Theoretically, the compiler should erase units of measures so that a the method from the sample code is a non-generic method that takes just an int.

Sample compiler outputs:
// The failing sample
namespace <StartupCode$FSharp-Test>
{
    internal static class $TestWithAnnotation
    {
        //
        // Nested Types
        //
        [Serializable]
        internal class g@10-1 : FSharpFunc<int, int>
        {
            public FSharp.Test.WithAnnotation.T this;

            internal g@10-1( FSharp.Test.WithAnnotation.T this )
            {
                this.this = this;
            }

            public override int Invoke( int x )
            {
                return this.this.H( x );
            }
        }
    }
}
// when manually erasing the UOM

// original code:
type T() =
    member this.H(x : int) = x

    member this.F(x : int) =
        let g x = this.H x

        g x // Raises System.BadImageFormatException


// fsc output:
namespace <StartupCode$FSharp-Test>
{
    internal static class $TestWithoutMeasure
    {
        //
        // Nested Types
        //
        [Serializable]
        internal class g@11-2 : FSharpFunc<int, int>
        {
            public FSharp.Test.WithoutMeasure.T this;

            internal g@11-2( FSharp.Test.WithoutMeasure.T this )
            {
                this.this = this;
            }

            public override int Invoke( int x )
            {
                return this.this.H( x );
            }
        }
    }
}
Concluding from this: The closures are correctly emitted, but someone decides there should be a .Specialize method to call.
Now trying to find the place where that is emited.... I think this should happen in publoc.fs which appears to perform some necessary closure transformations (I did originally think it was only called with --optimizations+ but that appears not to be the case).

JohannesRudolph wrote Sep 27, 2014 at 10:56 AM

I think pubclo.fs line 260 ff. are responsible for creating the call to .Specialize():
 let instrs1 = 
                        precall @
                        [ I_callvirt (doTailCall, 
                                      
                                      (mkILInstanceMethSpecInTy (cenv.mkILTyFuncTy,"Specialize",[],cenv.ilg.typ_Object, instTyargs)), None) ]
                    let instrs1 =                        
                        // TyFunc are represented as Specialize<_> methods returning an object.                            
                        // For value types, recover result via unbox and load.
                        // For reference types, recover via cast.
                        let rtnTy = mkTyOfApps cenv rest'
                        instrs1 @ [ I_unbox_any rtnTy]
I'll now trace up the call path to there. My gut feeling is that there is a mismatch between the decision logic in ilxgen (which at the very top has some code to "Drop" units of measurements from generic type parameter lists) and the code that possibly interpretes those same parameter lists in pubclo.fs.

I'm not sure I'm used to navigating in 5000 lines f# source files either.

JohannesRudolph wrote Sep 27, 2014 at 11:19 AM

What the heck codeplex. My code was beatufilly formatted 1s ago and then I reloaded and everything's broken? Also what are all these stupid
things for?

JohannesRudolph wrote Sep 27, 2014 at 11:37 AM

I'm getting closer. More braindump below. (Might hopefully help someone googling, would be nice though if someone could pick and confirm any of the guesses I'm making below):

ILX places IlxExt instructions in the instructions stream to indicate closure invocations.
pubclo.fs picks up on these two transfrom them into the final closure support class call sequence. It also splits the "abstract" Closure type definitions generated by ilx into multiple generated closures classes (for what purpose?). For each type argument and for each term argument, a separate closure class is generated. This sounds terribly complex to me but I bet it has some sort of advantage (well at least not at runtime because allocing so many closure classes is going to hurt performance).

Apps is probably a shortcut for "Application" (there are term=method argument and type=type applications).

In the meanwhile discovered that ILX stands for IL extended and originates from: http://research.microsoft.com/en-us/um/people/nick/entcs/syme.pdf
I think this paper should be very valuable for understanding what the codegen does (and why). Haven't read it yet though.
Why isn't this documented anywhere? E.g. put a comment on top of the ilx fail that links to that paper.

Found out that units of measurement are not properly erased in the call to GenArgsAndIndirectCall in ilxgen.fs (manually inserted a printf)
 GenArgsAndIndirectCall functy: forall _. (int<Microsoft.FSharp.Compiler.Tast+MeasureExpr+MeasureVar> -> int<Microsoft.FSharp.Compiler.Tast+MeasureExpr+MeasureVar>) tyargs: [Microsoft.FSharp.Compiler.Tast+MeasureExpr+MeasureVar] args: [Microsoft.FSharp.Compiler.Tast+Expr+Val] m: /Users/jr/dev/fsharp/lib/debug/test.fs (12,8--12,11) IsSynthetic=false

JohannesRudolph wrote Sep 27, 2014 at 11:39 AM

Please don't tell me Codeplex comments can only deal with CRLF line breaks.
in Unicode is LF. OMG. All my comment styling for null. We're in 2014 and I still have to deal with line ending issues. C'mon.

JohannesRudolph wrote Sep 27, 2014 at 12:25 PM

Wrapping up and concluding my research for now, I'm pretty sure that this is how the bug works:
  • The Methods H and F are incorrectly seen as generic methods because their type arguments not erased
  • I tried to erase the type arguments like this:
and GenApp cenv cgbuf eenv (f,fty,srcTyargs,args,m) sequel =
  printfn "GenApp tyargs before dropping: %A" srcTyargs
  let tyargs = DropErasedTyargs srcTyargs
  printfn "GenApp tyargs after dropping: %A" tyargs
But the resulting compiler would then crash with an internal compiler error when trying to compile the sample (no stacktrace or anything, just
/Users/jr/dev/fsharp/lib/debug/test.fs(10,13): error FS0073: internal error: expected a function type
Stack Trace
Microsoft.FSharp.Compiler.ErrorLogger+InternalError: Exception of type 'Microsoft.FSharp.Compiler.ErrorLogger+InternalError' was thrown.
I assume one of the things the code to drop the TypeArguments need to take care of as well is that the supposedly generic methods H and F become "non-generic" once all their units of measure type arguments have been erased.

Anyhow, I don't know enough about the internals of the compiler and can't justify spending more than the 6hrs that I've played with this, so I would really appreciate if someone more competent then me would take this over and fix it. We will stick with F#3.0 for the time being (because that doesn't crash at least in non-optimized debug builds and seems to work fine for us).

dsyme wrote Sep 28, 2014 at 8:40 PM

Hi all,

Yes, this sounds like it is on the right track.

I want to mention that it is my responsibility for originally considering this bug to be "very very rare" - I was mistaken. This led the bug to be unaddressed while other things were happening - the move to open source etc.

I personally will aim to make up for this by looking at the bug from here and getting a fix in as possible (if others don't get a fix in first - thing can happen fast around here!)

Cheers,
Don

latkin wrote Sep 28, 2014 at 9:28 PM

And as an FYI, we have contacted the Codeplex team about the newline encoding issues. They have identified the problem and will push out a fix in their next deployment.

JohannesRudolph wrote Sep 29, 2014 at 6:58 AM

@dsyme: Thanks a lot for taking care of this! I hope my research into this wasn't in vain, I learned a thing or two about the F# compiler in the process, and it's the first time I've been working with a huge F# codebase.

Ours is just a few thousand lines in small (< 500 LoC each) modules and makes heavy use of Units of Measurements as we work with a physical model. On a sidenote, I was wondering how "idiomatic" the use of such huge modules (> 5k lines) in the F# world is? What is neat about the modules in the F# Compiler is that the surface area (interfaces) is actually very thin. But I found navigating these huge files (without being easily able to lay out the code side by side rather than only top to bottom) very difficult.

@latkin: Thanks for forwarding it to the codeplex team and kudos for the fast turn around. I may have been a bit in a braindump/rant mode, so appologies if my comments were a bit sarcastic the other day.

dsyme wrote Oct 9, 2014 at 1:38 PM

@JohannesRudolph - if your code is available publicly I'd love to see it and reference it (and even add it as a test) - people often ask for examples of projects using UoM heavily. Also if you feel able to write a testimonial for http://fsharp.org/testimonials then please do consisder that.

@JohannesRudolph - re "idiomatic" - some of the files in the F# compiler are certainly too big. However, because this codebase has hsitorically been "duplicated" in a number of places (e.g. in the open edition of F#) and because there's a lot of active feature work going on, it has not been easy to split them up without messing with people's work. But it is true that a "large file + optional signature" is often used as a unit of encapsulation/componentization in F# and similar languages like OCaml. http://fsharpforfunandprofit.com has more info on this.

latkin wrote Oct 17, 2014 at 8:18 PM

PR has been applied, I can confirm fixed with 26073c3ec0

JohannDeneux wrote Jan 29, 2015 at 8:51 AM

I think I ran into this problem. The exceptions I got were different, I had NullReferenceException and MemoryAccessViloation (targetting framework 3.5, platform x86).

A simple work-around was to declare the offending closure as inline:
    let renderElement (gd : Graphics) (elementPos, elementBox) element labelLines =
        let dpcm = dotPerCm gd
        let inline dotify x = x * dpcm  // inline added to avoid incorrect codegen by the compiler in non-optimized builds.