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

IL generation is no longer deterministic #15048

Closed
nojaf opened this issue Apr 7, 2023 · 8 comments · Fixed by #15065
Closed

IL generation is no longer deterministic #15048

nojaf opened this issue Apr 7, 2023 · 8 comments · Fixed by #15065
Assignees
Milestone

Comments

@nojaf
Copy link
Contributor

nojaf commented Apr 7, 2023

Hello @KevinRansom,

After #14941, some IL seems to get embedded (g.TryEmbedILType) (at least for netstandard2.0).
The inserted location of .class private auto ansi beforefieldinit System.Runtime.CompilerServices.IsReadOnlyAttribute and .class private auto ansi beforefieldinit System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute does not appear to be deterministic.

image.

I believe this happens when in IlxGen.fs, GetReadOnlyAttribute and GetDynamicallyAccessedMemberTypes are called. They invoke cenv.g.TryEmbedILType which adds these new types.

Repro steps

Compile Fantomas.Core.fsproj using a local compiler. Do this multiple times

& "C:\Users\nojaf\Projects\fsharp\artifacts\bin\fsc\Release\net7.0\win-x64\publish\fsc.exe"
@Fantomas.Core.args.txt"

Note that this is a problem regardless of --test:GraphBasedChecking being active.

Decompiling using ILdasm seems to indicate the difference.

Fantomas.Core-01.txt
Fantomas.Core-02.txt
Fantomas.Core.args.txt

Expected behaviour

if

.class private auto ansi sealed System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes
       extends [netstandard]System.Enum
{
  .custom instance void [netstandard]System.FlagsAttribute::.ctor() = ( 01 00 00 00 ) 
  .custom instance void [netstandard]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) 
  .custom instance void [netstandard]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) 
  .field public specialname rtspecialname int32 value__ = int32(0x00000000)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes All = int32(0xFFFFFFFF)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes None = int32(0x00000000)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicParameterlessConstructor = int32(0x00000001)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicConstructors = int32(0x00000003)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicConstructors = int32(0x00000004)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicMethods = int32(0x00000008)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicMethods = int32(0x00000010)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicFields = int32(0x00000020)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicFields = int32(0x00000040)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicNestedTypes = int32(0x00000080)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicNestedTypes = int32(0x00000100)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicProperties = int32(0x00000200)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicProperties = int32(0x00000400)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes PublicEvents = int32(0x00000800)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes NonPublicEvents = int32(0x00001000)
  .field public static literal valuetype System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes Interfaces = int32(0x00002000)
} // end of class System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes

is expected to be embedded, that is fine. I don't really know the full details of this.
But it should be embedded as the first or last thing I guess. In some predicate way.

Actual behaviour

It can really jump around for me locally. Sometimes both attributes are mixed, sometimes only one of the two.

Known workarounds

None so far.

Related information

When I tried locally to force the call to g.TryEmbedILType by calling

    do ignore (GetReadOnlyAttribute cenv)
    do ignore (GetDynamicallyAccessedMemberTypes cenv)

sooner (in the constructor of IlxAssemblyGenerator) that didn't seem to have any effect.

@nojaf
Copy link
Contributor Author

nojaf commented Apr 11, 2023

I'm seeing this in Ubuntu as well:
Ubuntu.zip

@KevinRansom
Copy link
Member

@nojaf , I will take a look.

@nojaf
Copy link
Contributor Author

nojaf commented Apr 11, 2023

Thank you, let me know if there is anything I can do!

@KevinRansom KevinRansom self-assigned this Apr 11, 2023
@KevinRansom
Copy link
Member

It didn't actually occur to me that we do parallel ilxgen, so they get accumulated in different orders.

@KevinRansom
Copy link
Member

KevinRansom commented Apr 11, 2023

@nojaf, I believe the reason this occurs is because we emit the generated classes in the order the keys are accumulated in the concurrentdictionary. The fix will be to sort them in fullname order, before emitting them.

@KevinRansom
Copy link
Member

@yES, here we go, 3rd time it changed the order ..:

c:\kevinransom\fantomas\src\Fantomas.Core>"C:\Program Files\dotnet\dotnet.exe" c:\kevinransom\fsharp/artifacts/bin/fsc/Release/net7.0/fsc.dll @Fantomas.Core.rsp
Microsoft (R) F# Compiler version 12.5.0.0 for F# 7.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

warning FS1063: Unknown --test argument: 'ParallelCheckingWithSignatureFilesOn'
>>>>"System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes"<<<<
>>>>"System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute"<<<<

c:\kevinransom\fantomas\src\Fantomas.Core>"C:\Program Files\dotnet\dotnet.exe" c:\kevinransom\fsharp/artifacts/bin/fsc/Release/net7.0/fsc.dll @Fantomas.Core.rsp
Microsoft (R) F# Compiler version 12.5.0.0 for F# 7.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

warning FS1063: Unknown --test argument: 'ParallelCheckingWithSignatureFilesOn'
>>>>"System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes"<<<<
>>>>"System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute"<<<<

c:\kevinransom\fantomas\src\Fantomas.Core>"C:\Program Files\dotnet\dotnet.exe" c:\kevinransom\fsharp/artifacts/bin/fsc/Release/net7.0/fsc.dll @Fantomas.Core.rsp
Microsoft (R) F# Compiler version 12.5.0.0 for F# 7.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

warning FS1063: Unknown --test argument: 'ParallelCheckingWithSignatureFilesOn'
>>>>"System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute"<<<<
>>>>"System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes"<<<<

c

@KevinRansom
Copy link
Member

Okay, I have a fix for your issue, but I also see this:
''''
warning FS1063: Unknown --test argument: 'ParallelCheckingWithSignatureFilesOn'
The local method 'System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute'::'set_MemberType' was referenced but not declared
generic arity: 0
The local method 'System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute'::'set_Type' was referenced but not declared
generic arity: 0

This is unrelated, for now I will modify the code gen to make these properties public.  And start a new PR to not generate the setters in the first place.

@nojaf
Copy link
Contributor Author

nojaf commented Apr 12, 2023

Thanks for this investigation @KevinRansom!

Unknown --test argument: 'ParallelCheckingWithSignatureFilesOn' is to be expected, that got removed in #14494.

@github-project-automation github-project-automation bot moved this from Not Planned to Done in F# Compiler and Tooling Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants