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

4.1 compiler generates invalid programs for structs which use measure types #2373

Closed
BillHally opened this issue Feb 5, 2017 · 5 comments · Fixed by #2404
Closed

4.1 compiler generates invalid programs for structs which use measure types #2373

BillHally opened this issue Feb 5, 2017 · 5 comments · Fixed by #2404
Milestone

Comments

@BillHally
Copy link
Contributor

The combination of a struct, a measure type and no optimization can create incorrect programs which fail verification by peverify, and raise InvalidProgramException when run.

Repro steps

  1. Take the following code:

    type AStruct<[<Measure>]'u> = 
        struct
        end
    
    [<EntryPoint>]
    let main argv = 
        let x = AStruct()
        0
    
  2. Compile the code without optimization using version 4.1 of the F# compiler:

    PS> &"C:\Program Files (x86)\Microsoft SDKs\F#\4.1\Framework\v4.0\Fsc.exe" --optimize- .\Program.fs
    Microsoft (R) F# Compiler version 4.1
    Copyright (c) Microsoft Corporation. All Rights Reserved.
    
  3. Run the code:

    PS> .\Program.exe
    
    Unhandled Exception: System.InvalidProgramException: Common Language Runtime detected an invalid program.
       at Program.x@7..ctor()
       at Program.main(String[] argv)
    
  4. Verify the code:

    PS> peverify.exe Program.exe
    
    Microsoft (R) .NET Framework PE Verifier.  Version  4.0.30319.0
    Copyright (c) Microsoft Corporation.  All rights reserved.
    
    [IL]: Error: [...\Program.exe : Program::main][offset 0x00000005][found ref 'Program+x@7'][expected value 'Program+AStruct'] Unexpected type on the stack.
    [IL]: Error: [...\Program.exe : Program+x@7::.ctor][offset 0x00000009] Return from .ctor when this is uninitialized.
    2 Error(s) Verifying Program.exe
    

Expected behavior

Program runs without crashing, and verification with peverify completes without reporting any errors.

Actual behavior

The program crashes with an InvalidProgramException, and verification with peverify reports 2 errors.

Known workarounds

Either:

  1. Use a class instead of a struct
  2. don't use a measure type
  3. Turn on optimization

Related information

  • Operating system: Windows 10 (Anniversary Update)
  • .NET Runtime, CoreCLR or Mono Version: .Net 4.6.2
  • Indications of severity: prevents use of measure types with structs
@BillHally
Copy link
Contributor Author

This is related to 0fe9edd - reverting the changes it made prevents the errors described above from happening.

I'll investigate further tomorrow.

@dsyme
Copy link
Contributor

dsyme commented Feb 7, 2017

@cartermp This is a regression unfortunately

@cartermp cartermp added this to the VS 2017 Updates milestone Feb 7, 2017
@cartermp
Copy link
Contributor

cartermp commented Feb 7, 2017

Noted. Added to the list to document.

@BillHally
Copy link
Contributor Author

A few more observations:

  1. The original failing code above used an implicitly dimensionless unit of measure:

    type AStruct<[<Measure>]'u> = 
        struct
        end
    
    [<EntryPoint>]
    let main argv = 
        let x = AStruct() // This doesn't work
        0
  2. Implicitly using a measure type also fails:

    type AStruct<[<Measure>]'u> = 
        struct
            val X : float<'u>
        end
    
    [<Measure>] type u
    
    [<EntryPoint>]
    let main argv = 
        let x = AStruct() // This also doesn't work
        printfn "%g" (x.X + 1.0<u>)
        0
  3. However, explicitly specifying a dimensionless unit works:

    type AStruct<[<Measure>]'u> = 
        struct
        end
    
    [<EntryPoint>]
    let main argv = 
        let x = AStruct<1>() // This is fine
        0
  4. And explicitly providing a measure type works:

    type AStruct<[<Measure>]'u> = 
        struct
        end
    
    [<Measure>] type u
    
    [<EntryPoint>]
    let main argv = 
        let x = AStruct<u>() // This is fine
        0
  5. The reason that the explicit versions continue to work after the commit which added isLocalTypeFunc to the match expression ((1) below), is that they are passed as Const not TyLambda, so their path of execution is unchanged i.e. they pass through (2) below:

    and GenBindRhs cenv cgbuf eenv sp (vspec:Val) e =   
        match e with 
        | Expr.TyLambda _ | Expr.Lambda _ -> 
            let isLocalTypeFunc = IsNamedLocalTypeFuncVal cenv.g vspec e
            
            match e, isLocalTypeFunc (* (1) *) with
            | Expr.TyLambda(_, tyargs, body, _, _), true when 
                (
                    tyargs |> List.forall (fun tp -> tp.IsErased) &&
                    (match StorageForVal vspec.Range vspec eenv with Local _ -> true | _ -> false)
                ) ->
                // type lambda with erased type arguments that is stored as local variable (not method or property)- inline body
                GenExpr cenv cgbuf eenv sp body Continue
            | _ ->
                let selfv = if isLocalTypeFunc then None else Some (mkLocalValRef vspec)
                GenLambda cenv cgbuf eenv isLocalTypeFunc selfv e Continue 
        | _ -> // (2)
            GenExpr cenv cgbuf eenv sp e Continue

@liboz
Copy link
Contributor

liboz commented Feb 10, 2017

I looked into this a bit more. Here are some other ways that work to fix this:

type AStruct<[<Measure>]'u> = 
    struct
    end

[<EntryPoint>]
let main argv = 
    let x = fun () -> AStruct() // This works, you can also do: let x () = AStruct()
    printfn "%A" (x())
    0

or

type AStruct<[<Measure>]'u> = 
    struct
        val X : float<'u>
    end

[<Measure>] type u

[<EntryPoint>]
let main argv = 
    let x = AStruct().X + 1.0<u> // This also works
    0

As far as I can see it seems quite specific. I'm making an attempt to fix it here: #2404

KevinRansom pushed a commit that referenced this issue Feb 15, 2017
* attempt to fix units of measure issue with structs and no optimizations

* reducing code duplication

* keep the original comment

* add test
@cartermp cartermp modified the milestones: VS 2017 Updates, 15.2 Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants