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

Comments

Projects
None yet
4 participants
@BillHally
Contributor

BillHally commented Feb 5, 2017

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

This comment has been minimized.

Show comment
Hide comment
@BillHally

BillHally Feb 6, 2017

Contributor

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

I'll investigate further tomorrow.

Contributor

BillHally commented Feb 6, 2017

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

I'll investigate further tomorrow.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Feb 7, 2017

Contributor

@cartermp This is a regression unfortunately

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

This comment has been minimized.

Show comment
Hide comment
@cartermp

cartermp Feb 7, 2017

Collaborator

Noted. Added to the list to document.

Collaborator

cartermp commented Feb 7, 2017

Noted. Added to the list to document.

@BillHally

This comment has been minimized.

Show comment
Hide comment
@BillHally

BillHally Feb 8, 2017

Contributor

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
Contributor

BillHally commented Feb 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@liboz

liboz Feb 10, 2017

Contributor

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

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 KevinRansom closed this in #2404 Feb 15, 2017

KevinRansom added a commit that referenced this issue Feb 15, 2017

Fix for #2373 (#2404)
* 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