A function calling Assembly.GetExecutingAssembly() gets inlined across assembly boundaries

[ForNeVeR]

Repro steps

1. Create two projects: one exposing a function let getName() = Assembly.GetExecutingAssembly().GetName(), and another one using that function
2. Run the projects in Debug: name of the first project will be returned
3. Run the projects in Release: the call will be inlined, so the name of the second project will be returned

Here's a sample project with reproduction; just in case, I'll attach all the sources as a ZIP archive, too: fsharp-inlining-bug-master.zip

For ease of reading, here're the main source files:

// projecta/Program.fs
module ProjectA

open System.Reflection

let getName() =
    Assembly.GetExecutingAssembly().GetName()

// projectb/Program.fs
module ProjectB

open System.Reflection

let getName() =
    Assembly.GetExecutingAssembly().GetName()

[<EntryPoint>]
let main argv =
    printfn "Hello World from F#!"
    printfn "ProjectA: %A" <| ProjectA.getName()
    printfn "ProjectB: %A" <| getName()
    0

Output:

$ dotnet run --project projectb --configuration Debug
Hello World from F#!
ProjectA: projecta, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
ProjectB: projectb, Version=2.0.0.0, Culture=neutral, PublicKeyToken=null
$ dotnet run --project projectb --configuration Release
Hello World from F#!
ProjectA: projectb, Version=2.0.0.0, Culture=neutral, PublicKeyToken=null
ProjectB: projectb, Version=2.0.0.0, Culture=neutral, PublicKeyToken=null

Expected output: in both cases, the same:

Hello World from F#!
ProjectA: projecta, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
ProjectB: projectb, Version=2.0.0.0, Culture=neutral, PublicKeyToken=null

Known workarounds

Mark a function containing a call to Assembly.GetExecutingAssembly() as [<MethodImpl(MethodImplOptions.NoInlining)>].

Related information

• Operating system: Windows 10
• .NET Runtime: .NET Core SDK 3.1.201

[abelbraaksma]

I think this is a feature and not a bug. Assembly.GetExecutingAssembly() is defined to walk the stack to find out where it is. If functions get inlined, either by JIT (which I think happens here) or F# (when using inline implicitly or explicitly), the stack changes, and so does the result of this call. This is also suggested in this SO answer from 2013: https://stackoverflow.com/a/19845144/111575.

In my local logging framework I exploit this behavior to get the executing assembly from the call to the log, not the log assembly itself. Changing this behavior would break that code and possibly others that use similar techniques.

You probably want to use Type.GetExecutingAssembly instead. It doesn't have the same issue, I believe.

[ForNeVeR]

@abelbraaksma

If functions get inlined, either by JIT (which I think happens here)

No, in this case, the function is inlined by the F# compiler; I was able to confirm this hypothesis by reading the decompiled code of my assembly. The entry point looks like this:

    public static int main(string[] argv)
    {
        PrintfFormat<Unit, TextWriter, Unit, Unit> printfFormat = new PrintfFormat<Unit, TextWriter, Unit, Unit, Unit>("Hello World from F#!");
        PrintfModule.PrintFormatLineToTextWriter<Unit>(Console.Out, printfFormat);
        PrintfFormat<FSharpFunc<AssemblyName, Unit>, TextWriter, Unit, Unit> printfFormat2 = new PrintfFormat<FSharpFunc<AssemblyName, Unit>, TextWriter, Unit, Unit, AssemblyName>("ProjectA: %A");
        PrintfModule.PrintFormatLineToTextWriter<FSharpFunc<AssemblyName, Unit>>(Console.Out, printfFormat2).Invoke(Assembly.GetExecutingAssembly().GetName());
        printfFormat2 = new PrintfFormat<FSharpFunc<AssemblyName, Unit>, TextWriter, Unit, Unit, AssemblyName>("ProjectB: %A");
        PrintfModule.PrintFormatLineToTextWriter<FSharpFunc<AssemblyName, Unit>>(Console.Out, printfFormat2).Invoke(Assembly.GetExecutingAssembly().GetName());
        return 0;
    }

Here, the first call to Assembly.GetExecutingAssembly().GetName() is, in fact, inlined code from another F# assembly.

Also, I believe that Assembly.GetExecutingAssembly is marked by a special attribute (System.Security.DynamicSecurityMethod) to avoid this JIT behavior:

All methods that use StackCrawlMark should be marked with this attribute. This attribute disables inlining of the calling method to allow stackwalking to find the exact caller.

It may or may not be a problem that I wasn't able to find DynamicSecurityMethod attribute on the Assembly.GetExecutingAssembly in the ref assemblies I (seemingly) compile my .NET Core code against.

I mean, if F# compiler starts respecting this attribute at some point (currently it seems it doesn't), its absence from the ref assemblies may impose a problem.

In my local logging framework I exploit this behavior to get the executing assembly from the call to the log, not the log assembly itself.

How do you work around the fact that this approach doesn't work in Debug mode?

You probably want to use Type.GetExecutingAssembly instead. It doesn't have the same issue, I believe.

You probably meant Type.GetAssembly(), and, yes, this is a viable work around this particular problem, too. But sometimes this itself may be a problem, since it's not that easy to get any meaningful type from an F# assembly, e.g. typeof<MyModule> doesn't work.

To conclude, currently I believe this is, in fact, a bug and not a feature. F# compiler inlines the code that shouldn't be inlined, as described by DynamicSecurityMethod documentation, which leads to unreliable runtime behavior (different between debug and release builds). Any method calling a method marked with DynamicSecurityMethod shouldn't be inlined — either by JIT, or by the F# compiler (since it moslty respects attributes used to control JIT).

Not a big deal? Probably. A feature? Nope.

[KevinRansom]

@ForNeVeR To disable compiler inlining use: [<MethodImpl(MethodImplOptions.NoInlining)>]

Try this:

// projecta/Program.fs
module ProjectA

open System.Reflection
open System.Runtime.CompilerServices

[<MethodImpl(MethodImplOptions.NoInlining)>]
let getName() =
    Assembly.GetExecutingAssembly().GetName()

Produces this output on retail build:

Hello World from F#!
ProjectA: ClassLibrary1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
ProjectB: ConsoleApp5, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null

[abelbraaksma]

How do you work around the fact that this approach doesn't work in Debug mode?

@ForNeVeR, I was wrong, the code uses Assembly.GetCallingAssembly instead, and to find the line number it injects a dummy function (and extracts the line number from that), and further some magic to distinguish between lambdas and local functions, to get the relevant function name.

All these methods are marked debug-only, so I'm doing the inverse: when in debug mode, I know certain functions are not erased or inlined (unless inline is present) and leverage that info. The release-mode log functions don't do this, as the same trick wouldn't work.

Sorry for the clutter, as I deliberately wrote in my code not to use Assembly.GetExecutingAssembly. I searched for the places where I do use it, and these are all module-level let-bindings. Since these are never inlined anyway, and will only execute once, this gives me the right assembly (which is, btw, another way to work around the inlining issue).

Not a big deal? Probably. A feature? Nope.

Totally in agreement there now, thanks for the insights!

[ForNeVeR]

@KevinRansom, thanks, I confirm that this workaround works (and already mentioned it in my opening post). I do still believe the current behavior is an issue, but I don't think it's a critical one.

[KevinRansom]

If you don't mind, I'm going to close this, everything is working as it is intended.

Thanks for the report.

[abelbraaksma]

@KevinRansom, I don't think it is working as expected.

You wrote:

To disable compiler inlining use: [<MethodImpl(MethodImplOptions.NoInlining)>]

But in this case, the method Assembly.GetExecutingAssembly is actually marked with precisely that, but this isn't honored:

[MethodImpl(MethodImplOptions.NoInlining)]
[SecuritySafeCritical]
[__DynamicallyInvokable]
public static Assembly GetExecutingAssembly()
{
    StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
    return RuntimeAssembly.GetExecutingAssembly(ref stackMark);
}

And:

    // DynamicSecurityMethodAttribute:
    //  All methods that use StackCrawlMark should be marked with this attribute. This attribute
    //  disables inlining of the calling method to allow stackwalking to find the exact caller.
    //
    //  This attribute used to indicate that the target method requires space for a security object
    //  to be allocated on the callers stack. It is not used for this purpose anymore because of security
    //  stackwalks are not ever done in CoreCLR.

And I can confirm, by looking at the generated IL from the FSC compiler that @ForNeVeR is correct that the method that uses GetExecutingAssembly gets inlined indeed, and the NoInlining from the BCL library seems to have not been taken into account.

Yes, the workaround is to add NoInlining to your own function, but surely, that oughtn't be needed? Or I'm missing something in how NoInlining is supposed to work.

[ForNeVeR]

I agree with the previous comment. It looks like this is a compiler bug (or, let's say, an open issue in compiler), even if a minor one. I think it should be reopened, if there's no policy to close minor/archived/postponed bugs.

[cartermp]

Yeah, this looks like a bug.

[ForNeVeR]

@abelbraaksma, I have a comment on your understanding of NoInlining though. As it seems (to me; feel free to correct if I'm wrong!), it isn't, and shouldn't be applied transitively. Applying NoInlining to a method doesn't mean that any methods calling it shouldn't be inlined; otherwise, a call to a method with NoInlining would momentarily infect the whole code base, require additional heavy tracking from compiler/JIT etc.

NoInlining means that the method attributed with it shouldn't be inlined, and that's all. It doesn't mean that its callers automatically get NoInlining, too.

DynamicSecurityMethod grants different behavior though: from the comment it looks like its callers should effectively get NoInlining (which is exactly the sort of behavior that's required for my sample to reliably work independent of debug/release settings).

[abelbraaksma]

@ForNeVeR Ok, reading again al comments, trying to understand. You wrote:

All methods that use StackCrawlMark should be marked with this attribute.

But GetExecutingAssembly is not marked with that attribute, even though it uses StackCrawlMark (which is just an enum, but looks like it's used to create a point in the stack). This may be a BCL bug.

Then there's NoInlining. You say it's not transitive. So if A calls B, and C calls A, then if B is marked NoInlining, and A is not, method A can be inlined, which leads to its call being erased and it'll look like C calling B directly. Just like your first example, and your decompiled code shows.

If it's not transitive, and the proper security attribute is not present on GetExecutingAssembly, then fsc is doing what it should do (even though it's not expected), and the bug is with the library method not being marked correctly.

This is my educated analysis, being on mobile I didn't check this with any spec.

[ForNeVeR]

@abelbraaksma, I can see your logic, but it looks like we're looking at different sources/variants of GetExecutingAssembly. Let's compare them?

For definition of GetExecutingAssembly, I am looking here: https://github.com/dotnet/runtime/blob/c9f917052fc091014ef0f311b6f79a812bbfbd38/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Assembly.CoreCLR.cs#L69-L73

(which could or could not be the real sources used by runtime)

These more or less correspond to the sources I get from the source server when looking for Assembly.GetExecutingAssembly from netcoreapp3.1 using Rider (which is able to use SourceLink or whatever this great technology is named currently, to gather the sources of used assemblies automatically).

And where did you get the sources without this attribute, but with different attribute set?

[abelbraaksma]

but it looks like we're looking at different sources/variants of GetExecutingAssembly. Let's compare them?

You were right. The quote for the source of GetExecutingAssembly that I showed was after compiling a bit of example code for .NET Framework and inspecting it with ILSpy. In such case, the referenced assemblies in the source are reference assemblies without code. After assembly-resolution, this resolves to mscorlib, which has this:

.method public hidebysig static 
    class System.Reflection.Assembly GetExecutingAssembly () cil managed noinlining 
{
    .custom instance void System.Security.SecuritySafeCriticalAttribute::.ctor() = (
        01 00 00 00
    )
    .custom instance void __DynamicallyInvokableAttribute::.ctor() = (
        01 00 00 00
    )
    // Method begins at RVA 0xf6184
    // Code size 10 (0xa)
    .maxstack 1
    .locals init (
        [0] valuetype System.Threading.StackCrawlMark
    )

    IL_0000: ldc.i4.1
    IL_0001: stloc.0
    IL_0002: ldloca.s 0
    IL_0004: call class System.Reflection.RuntimeAssembly System.Reflection.RuntimeAssembly::GetExecutingAssembly(valuetype System.Threading.StackCrawlMark&)
    IL_0009: ret
} 

Contrast that with .NET Core 3.1, which has this (from ILSpy):

.method public hidebysig reqsecobj static 
    class System.Reflection.Assembly GetExecutingAssembly () cil managed 
{
    // Method begins at RVA 0x159270
    // Code size 10 (0xa)
    .maxstack 1
    .locals (
        [0] valuetype System.Threading.StackCrawlMark
    )

    IL_0000: ldc.i4.1
    IL_0001: stloc.0
    IL_0002: ldloca.s 0
    IL_0004: call class System.Reflection.RuntimeAssembly System.Reflection.Assembly::GetExecutingAssembly(valuetype System.Threading.StackCrawlMark&)
    IL_0009: ret
}

~Note that in my version of .NET Core, I don't have System.Security.DynamicSecurityMethod, which is surprising, esp since it seems to be in the source at your link since 2017 or so. I don't know why the compiled version has this attribute omitted.~ EDIT: it was hidden, it is reqsecobj which is a reserved Method Flag when the CoreLib source uses DynamicSecurityMethod. It does not show as that attribute in decompiled assemblies.

---

FSharpBug9283.zip

I created a little test solution to compare, this is from VS 2019, I didn't try Rider. It has two libraries, one for .NET Core and one for .NET Framework. And it has four console apps, two for each library in C# and F#. The library looks like this (same for Core and Framework, only namespace and module name differ, added a bunch of variants to compare consistency of inlining for different types of methods):

namespace FSharpBug9283_NetCore

open System
open System.Reflection
open System.Runtime.CompilerServices

module Core = 
    module ModGetAsm =
        let staticName = Assembly.GetExecutingAssembly().GetName().Name

        let getName() =
            let asm = Assembly.GetExecutingAssembly().GetName().Name
            asm

    type GetAsm() = 
        static let staticName = Assembly.GetExecutingAssembly().GetName().Name

        let staticName = Assembly.GetExecutingAssembly().GetName().Name

        let getName() =
            let asm = Assembly.GetExecutingAssembly().GetName().Name
            asm

        static member GetName() =
            let asm = Assembly.GetExecutingAssembly().GetName().Name
            asm

        member _.InstanceGetNameIndirect1() = getName()

        member _.InstanceGetNameIndirect2() = staticName

        static member GetNameIndirect3() = staticName

        member _.InstanceGetName() =
            let asm = Assembly.GetExecutingAssembly().GetName().Name
            asm

And this is the F# calling code:

open FSharpBug9283_NetCore

[<EntryPoint>]
let main argv =

    printfn "\n.NET Core: "
    printfn "Module value:            %s" Core.ModGetAsm.staticName
    printfn "Module function:         %s" (Core.ModGetAsm.getName())
    printfn "Static class member:     %s" (Core.GetAsm.GetName())
    printfn "Static class member ind: %s" (Core.GetAsm.GetNameIndirect3())
    printfn "Instance member:         %s" (Core.GetAsm().InstanceGetName())
    printfn "Instance member ind 1:   %s" (Core.GetAsm().InstanceGetNameIndirect1())
    printfn "Instance member ind 2:   %s" (Core.GetAsm().InstanceGetNameIndirect2())

And this is the C# calling code:

using FSharpBug9283_NetCore;
using System;

namespace FSharpBug9283_NetCore_Caller_CSharp
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("\n.NET Core: ");
            Console.WriteLine("Module value:            {0}", Core.ModGetAsm.staticName);
            Console.WriteLine("Module function:         {0}", Core.ModGetAsm.getName());
            Console.WriteLine("Static class member:     {0}", Core.GetAsm.GetName());
            Console.WriteLine("Static class member ind: {0}", Core.GetAsm.GetNameIndirect3());
            Console.WriteLine("Instance member:         {0}", new Core.GetAsm().InstanceGetName());
            Console.WriteLine("Instance member ind 1:   {0}", new Core.GetAsm().InstanceGetNameIndirect1());
            Console.WriteLine("Instance member ind 2:   {0}", new Core.GetAsm().InstanceGetNameIndirect2());
        }
    }
}

If I run all combinations (F# vs C# vs Debug vs Release), this is the output (which backs your original report):

######### F# ###########

DEBUG BUILD .NET Framework:
Module value:            FSharpBug9283_NetFramework
Module function:         FSharpBug9283_NetFramework
Static class member:     FSharpBug9283_NetFramework
Static class member ind: FSharpBug9283_NetFramework
Instance member:         FSharpBug9283_NetFramework
Instance member ind 1:   FSharpBug9283_NetFramework
Instance member ind 2:   FSharpBug9283_NetFramework

RELEASE BUILD .NET Framework:
Module value:            FSharpBug9283_NetFramework
Module function:         FSharpBug9283_NetFramework_Caller
Static class member:     FSharpBug9283_NetFramework_Caller
Static class member ind: FSharpBug9283_NetFramework
Instance member:         FSharpBug9283_NetFramework_Caller
Instance member ind 1:   FSharpBug9283_NetFramework
Instance member ind 2:   FSharpBug9283_NetFramework

DEBUG BUILD .NET Core:
Module value:            FSharpBug9283_NetCore
Module function:         FSharpBug9283_NetCore
Static class member:     FSharpBug9283_NetCore
Static class member ind: FSharpBug9283_NetCore
Instance member:         FSharpBug9283_NetCore
Instance member ind 1:   FSharpBug9283_NetCore
Instance member ind 2:   FSharpBug9283_NetCore

RELEASE BUILD .NET Core:
Module value:            FSharpBug9283_NetCore
Module function:         FSharpBug9283_NetCore_Caller
Static class member:     FSharpBug9283_NetCore_Caller
Static class member ind: FSharpBug9283_NetCore
Instance member:         FSharpBug9283_NetCore_Caller
Instance member ind 1:   FSharpBug9283_NetCore
Instance member ind 2:   FSharpBug9283_NetCore

######### C# ###########

DEBUG BUILD .NET Framework:
Module value:            FSharpBug9283_NetFramework
Module function:         FSharpBug9283_NetFramework
Static class member:     FSharpBug9283_NetFramework
Static class member ind: FSharpBug9283_NetFramework
Instance member:         FSharpBug9283_NetFramework
Instance member ind 1:   FSharpBug9283_NetFramework
Instance member ind 2:   FSharpBug9283_NetFramework

RELEASE BUILD .NET Framework:
Module value:            FSharpBug9283_NetFramework
Module function:         FSharpBug9283_NetFramework
Static class member:     FSharpBug9283_NetFramework
Static class member ind: FSharpBug9283_NetFramework
Instance member:         FSharpBug9283_NetFramework
Instance member ind 1:   FSharpBug9283_NetFramework
Instance member ind 2:   FSharpBug9283_NetFramework

DEBUG BUILD .NET Core:
Module value:            FSharpBug9283_NetCore
Module function:         FSharpBug9283_NetCore
Static class member:     FSharpBug9283_NetCore
Static class member ind: FSharpBug9283_NetCore
Instance member:         FSharpBug9283_NetCore
Instance member ind 1:   FSharpBug9283_NetCore
Instance member ind 2:   FSharpBug9283_NetCore

RELEASE BUILD .NET Core:
Module value:            FSharpBug9283_NetCore
Module function:         FSharpBug9283_NetCore
Static class member:     FSharpBug9283_NetCore
Static class member ind: FSharpBug9283_NetCore
Instance member:         FSharpBug9283_NetCore
Instance member ind 1:   FSharpBug9283_NetCore
Instance member ind 2:   FSharpBug9283_NetCore

I should add that if I add AggressiveInlining to each method in the libraries, C# nor JIT inlines anything! If I add NoInlining, F# behaves as expected and both C# and F# behaviors are equal.

I'll add my concluding thoughts in the next post.

[abelbraaksma]

Here's what I gathered from my attempt to find definitive™ references:

• C# does not do static inlining at source-level, F# does, which explains the differences
• The JIT does not honor AggressiveInlining, this seems correct, see https://github.com/mono/mono/pull/13906
• As you said, NoInlining is not transitive, but security-criticality is
• Attributes on GetExecutingAssembly are not consistent between .NET versions:
  • .NET Framework has the NoInlining flag (method impl flag 0x0008), but not reqsecobj
  • .NET Core has the reqsecobj (which is a reserved flag in the method flags, 0x8000, as a result of DynamicSecurityMethodAttribute), but not the NoInlining flag.
• There's a special rule in the JIT that checks for reqsecobj. The rule seems to be that if the local vars contain a StackCrawlMark, and whether it is used as a reference in a call to another method. If both are true, the method must not be inlined, Any method that uses this, must also set reqsetobj, see https://github.com/dotnet/coreclr/blob/606c246/src/System.Private.CoreLib/src/System/Type.CoreCLR.cs#L38-L43.
• The "trick" with StrackCrawlMark appears to follow from this requirement, from the book .NET 2.0 IL Assembler, p189:

  reqsecobj means This method calls another method containing security code, so it requires an additional stack slot for a secuirity object. This flag is formally under the Reserved mask, so it cannot be set explicitly. Setting this flag requires emitting the pseudocustom attribute DynamicSecurityMethodAttribute.

• There are many more methods like GetExecutingAssembly that fall into the same category, see https://github.com/dotnet/runtime/issues/11698#issuecomment-449225852.
• There seems to be an effort to make these stacks less deep, or remove the requirement completely in cases where it is not necessary, see https://github.com/dotnet/runtime/issues/11698 and https://github.com/dotnet/coreclr/pull/21783.

I also found additional information through this excellent blog post on stack-crawling: https://mattwarren.org/2019/01/21/Stackwalking-in-the-.NET-Runtime/.

---

If I try to put all this scattered info together, I think that:

1. Inlining, either at source level by the compiler, or at JIT time should not be done when reqsecobj is present.
2. For F# I think that should mean that unless we must inline (i.e. for SRTP etc), we shouldn't inline if this attribute is present on a member in the body of a function or method, as it means it is dependent on the caller context.
3. But... a counter-argument can be: a rigid reading of all of the above doesn't seem to require this in user-code, since this attribute is reserved, it can only appear in Core libs anyway. However, the JIT doesn't inline the user-code in any case, though normally it would, and not even if you try to force it with AggressiveInlining. So I'm still convinced we shouldn't inline such cases either.

Whatever we choose to do, there's still the inconsistency between .NET versions...

[ForNeVeR]

@abelbraaksma, thank you so much for your investigation. I have tried to inspect the code earlier using reflection, and was puzzled that I cannot find any trace of that DynamicSecurityMethodAttribute. I was suspecting it's represented by some IL flag, but had no enough time to determine which one. Now we know it's reqsecobj, which I was able to confirm using this snippet on sharplab.

• As you said, NoInlining is not transitive, but security-criticality is

I can't help but to argue, or either clarify this a bit more: as I can see, security-criticality in only transitive one level down, e.g. a method calling a method calling a security-critical method shouldn't get any unusual properties, while a direct caller of a security-critical method should.

(this is very important for ease of implementation, that's why I'm nitpicking here)

Other than that, I completely agree with your analysis. And yes, there seem to be a problem with the reference assemblies that carry no such attribute. Probably Roslyn (or whatever party is used to generate reference assemblies) should preserve it?

[abelbraaksma]

I can't help but to argue, or either clarify this a bit more: as I can see, security-criticality in only transitive one level down,

I actually had different arguments/conclusions before I wrote down that simple bullet point in the end, I expected a response there ;).

If you look at how stuff for mscorlib is implemented, you can see that if a method sets reqsecobj, the next method (calling method) will typically set this too. But this may only be correct inside mscorlib (and by extension, any implementation of .NET Core as well). At the same time, there is the ongoing effort of lessening this increased stack-pressure (see links).

I think you are right that this transitivity doesn't carry over to user-code, as I assumed somewhat in my 3rd point at the bottom. In which case indeed only one level of non-inlining needs to be preserved to keep the stack-crawl marks in the right place.

and was puzzled that I cannot find any trace of that DynamicSecurityMethodAttribute

Yeah, same for me, it wasn't until I read the IL Assembler book sections that it became clear. You'd expect this to be in the docs, at least as a remark or something. There's surprisingly little info about this out there, on any platform.

And yes, there seem to be a problem with the reference assemblies that carry no such attribute.

Actually, the IL-disassembly I copied is not from the reference assemblies, that's from the resolved assemblies. But the difference between .NET Framework and .NET Core is real and I'm not sure what the best cause of action is. Probably @dsyme or @cartermp should chime in for that (both, see https://github.com/dotnet/fsharp/issues/9283#issuecomment-636508661 and further for the start of the more detailed analysis of this).

[dsyme]

@abelbraaksma Great analysis

Inlining, either at source level by the compiler, or at JIT time should not be done when reqsecobj is present. For F# I think that should mean that unless we must inline (i.e. for SRTP etc), we shouldn't inline if this attribute is present on a member in the body of a function or method, as it means it is dependent on the caller context.

Is this attribute present in the .NET Standard 2.0 reference assemblies? (What's in the implementation assemblies isn't of much help as the compiler doesn't see those). All I'm seeing is this (this is for C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\3.1.0\ref\netcoreapp3.1\System.Runtime.dll)

.method public hidebysig static class System.Reflection.Assembly 
        GetExecutingAssembly() cil managed
{
  // Code size       2 (0x2)
  .maxstack  8
  IL_0000:  ldnull
  IL_0001:  throw
} // end of method Assembly::GetExecutingAssembly

In the absence of a usable attribute I guess we should just make a list of everything on the core libraries that is sensitive to its current caller and hardwire that into the compiler.

[abelbraaksma]

Is this attribute present in the .NET Standard 2.0 reference assemblies?

@dsyme It appears to be non-present. I tried a handful of reqsecobj (i.e., where the implementation assembly has it set) and then checked the corresponding reference assembly, which does not have it set.

Not sure this is an oversight of the runtime team or whether this omission is by design or not.

[KevinRansom]

@dsyme ---- let's not have a list of non-inlinable apis in the compiler. I think this falls into the camp of it hurts stop doing it. I can as easily imagine scenarios, where the inlinining is desired as I can imagine those where it is not desired.

We should just ensure that the inlining behaviour is called out to developers. If we were the C# team we would probably say let's build an analyzer that spots this. An analyzer can have as much hired wired as it wants. So perhaps add a check to fslint?

[abelbraaksma]

I can as easily imagine scenarios, where the inlinining is desired as I can imagine those where it is not desired.

@KevinRansom, please note that reqsecobj is an attribute that is required to be followed, and implementations that cannot do that are therefore not compatible, basically violating the CLR spec. It also changes behavior, as can be seen in examples here: https://github.com/dotnet/fsharp/issues/9283#issuecomment-636508661.

If we were the C# team we would probably say let's build an analyzer that spots this.

The C# team just recognizes reqsecobj properly: using AggressiveInlining etc will never inline such methods.

The problem we are facing is that we don't "see" this because we link the reference assemblies during compilation, where this attribute is not exposed. I've reported it to .NET in case it's considered an oversight (any other meta information is available in the reference assemblies), see https://github.com/dotnet/runtime/issues/41690.

[KevinRansom]

@abelbraaksma , we are most assuredly not violating the CLR spec, because we are inlining IL at source code compile time. The spec is referring to optimizing the native code that is generated at jit or native compilation time. It might be worth getting the BCL team to add the attribute to reference assemblies or otherwise decorate them so that we have a chance to deal with it, I am merely reluctant to add another list of "special types that we treat differently".

The reason that C# doesn't do this, is because they rely on the jit for inlining operations and we don't.

[abelbraaksma]

we are most assuredly not violating the CLR spec, because we are inlining IL at source code compile time. The spec is referring to optimizing the native code that is generated at jit or native compilation time.

Aha, so you're basically saying that even though the JIT is not allowed to inline, we can, because we do it at compile time? I didn't look at it that way.

But still, it creates the awkward situation as explained by the OP: if we do things the JIT cannot do, this subsequently leads to differences in runtime result. This is particularly painful for methods that heavily rely on an intact stack frame.

It might be worth getting the BCL team to add the attribute to reference assemblies or otherwise decorate them so that we have a chance to deal with it, I am merely reluctant to add another list of "special types that we treat differently".

I fully agree, and hopefully @jkotas will give it some consideration, but it looks like there may be another solution (I just don't see how it helps us, yet): https://github.com/dotnet/runtime/issues/41690#issuecomment-685144368

[KevinRansom]

@abelbraaksma , in FSharp inline does exactly what you would expect. It inlines the code and inlines much bigger chunks of code than the jit does. If we want the compiler to respect a set of attributes that are only in the implementation assemblies, then the dotnet framework team need to ensure those methods are available to us at compile time. And then we need to implement the feature that enables it.

It should be noted, that turning that feature on would be a breaking change. It should also be noted that just switching from debug to retail today would also change behaviour.

If the developer wanted that Api to not behave as though it was not inlined, then placing inline on the method that included it is a bug. I may be looking at this too simplistically ... but I don't think so.

Kevin

[abelbraaksma]

@KevinRansom, this issue is not about using the inline keyword. It's about the optimization phase of F# where it inlines simple methods kinda just like the JIT does.

Hence the observed difference of behavior, even though the method signatures used in F# vs C# were equal and no inline, implicitly or explicitly, is used.

The problem arises when the body of such a method contains a method call that relies in the stack. When that stack gets erased, outside of the control of the user, it leads to surprises and undocumented behavior (as in, contrary to what the method is described to do).

[KevinRansom]

Then shove the no-inlining attribute on the method, and call it good.

[abelbraaksma]

Indeed, that's mentioned above as workaround ;).

But that's rather hard to diagnose, most people are not aware of this, so it's better to try to make the compiler behave "more correctly" in light of such methods. They are marked for a reason with the (little known) reqsecobj which is meant to ensure a stack frame is intact and not erased.

[KevinRansom]

@abelbraaksma ... it's not really that hard to diagnose, the hard part is you need to see it in testing and decide what you really want to do at that point. Again I would suggest add something to fslint that requests the developer look at it and decide what to do, and move on.

That API is marked with that attribute for jit compilation, I wonder what the source generators are going to do with this on the AOT scenarios, I imagine they will say to the bcl team decorate it in the reference assemblies and we will honour it somhow, also I suppose the source generator will be generating a new assembly, so inlining is less likely, although I can imagine doing it.

The compile time assemblies don't have it, and until they do any other heuristic we pick is just that, a heuristic, and its likely to break over time. Step one is for the framework guys to decorate those assemblies. Heuristics and well known lists are particularly annoying, to me this week because when I ensured FSI and the compiler could support netstandard assemblies, I broke support for all older .net frameworks which didn't have the netstandard shim in their references assemblies, it's taken until now to show up, however it did eventually and so now I have to fix it and I am not sure how.

Having said that, this feels pretty much a consequence of us opportunistically optimizing methods, that has other side effects for example when throwing an exception from an inlined function the stack frame doesn't include the function that was inlined only the call site to which it was inlined.

I assert this specific issue is rare, it is not at all certain that a developer wants to report the name of the dll the code was inlined from. I would actually like the caller name in logging scenarios, of course in that case because I want it, I would certainly put inline on it to make it clear, and I would be really annoyed if it didn't do what I wanted.

Kevin

[abelbraaksma]

I think we agree. If inline is used explicitly, we have no choice but to honor it, that's the way it's always been for F# and none of us wants to change that. If we optimize and inline small methods not marked inline, we should use a deterministic other way to find out whether it's eligible or not. That should be the reference assemblies...

While these don't show that information, unless there's another analytical method we can use (like using the implementation assemblies), we shouldn't hard code this in, as that's prone to maintenence nightmare.

[KevinRansom]

@abelbraaksma , we can't use the implementation assemblies, there is no guarantee that the flag will remain constant, and transfer across platforms. Although to be fair, for this example it is likely.

Even if the above wasn't true it is still the case that modifying the behavior is still a breaking change. Or at least a change in behavior. It is just as likely I think that a developer would expect the behavior to not change in the future. If I wrote the logging api that depended on automagic inlining that I hypothesized before I would be annoyed that I had to change my code to now say inline on the code that had inlined perfectly well before.

This is one of those changes that one would like to fix from a position of consistency or completeness, but the question is "Is it actually worth the change"?

Right now the behavior is deterministic discoverable, and using inline and the dontinline attribute can even be made explicit. I strongly suggest using fslint to prompt developers to use either the dontinline attribute or the inline flag.

We should also consider, including fslint and configuration property pages in VS and Ionide by default.

[baronfel]

For what it's worth, fsharplint can be configured through a json file and FSAC-based editors do pick that up. No nice UI, but json for these sorts of linting tools is pretty commonplace (eslint being the initial driver).

[teo-tsirpanis]

If we optimize and inline small methods not marked inline, we should use a deterministic other way to find out whether it's eligible or not.

And I was ready to create an issue about that...

My opinion is that inlining functions at the compiler-level without the inline keyword is a bad idea. It affects generated code size and such automatic inlining for the purposes of optimization is the job of the JIT, not the compiler. It also widens the gap between the IL code the developer expects to be generated, with the one that gets actually generated.

However, such automatic inlining looks OK for limited cases like small nested functions or active patterns.

Automatically inlining functions across assembly boundaries, like what this issue is concerned with, is a terrible idea because it breaches the barrier of abstraction between assemblies and should never be performed without the inline keyword.

[KevinRansom]

@teo-tsirpanis -- I don't know that I agree that it is a terrible idea, however, it is often quite inconvenient, and makes the source code a lot harder to reason about. I think that having fslint tell developers this is an iffy construct and they should use the noinlining attribute is the best result we can do right now. For sure turning off automagic inlining would ensure that the performance characteristics of most code would change, and probably not for the better.

[jl0pd]

I agree with Teo that automatic cross-assembly inlining is bad idea. Code tends to change and once correct code can cause MissingMethodException/TypeLoadException or old/incorrect behavior. Example: application with plugins - you can't control all code and recompile every library. As developer you don't change public api, but F# compiler goes through public methods and can inline internal/private code.

Maybe compiler should have flag to control implicit inlining, e.g. none, projectReferences, all: <ImplicitInlines>none</ImplicitInlines>. By default set it to projectReferences as safe version, but people could extend it to all references to improve performance

[dsyme]

You can disable cross-assembly inlining using --crossoptimize-

Libraries can suppress cross-assembly inline information using --nooptimizationdata

[abelbraaksma]

The dotnet bug report led to a new API proposal that would allow to implicitly pass caller identity, which would ultimately be an alternative for the flawed behavior explained in this issue, see: https://github.com/dotnet/csharplang/issues/4984.

[KevinRansom]

I am going to close this, there is nothing really for us to do. To address this a developer can add [<MethodImpl(MethodImplOptions.NoInlining)>], there's no reliable attribute for us to detect, and lists of well-known APIs are always difficult to reason about and maintain, and the developer can also suppress cross-assembly inlining when needed.

Thanks for the discussion

Kevin

[abelbraaksma]

To address this a developer can add [<MethodImpl(MethodImplOptions.NoInlining)>],

@KevinRansom, the point of this whole discussion and issue was, that NoInlining does not work, except only partially. Unless that has been fixed meanwhile. I.e.:

See this comment for a summary: https://github.com/dotnet/fsharp/issues/9283#issuecomment-685060334 and this:

As you said, NoInlining is not transitive, but security-criticality is

I'm not saying it is wrong to close this, I consider it "not worth the effort" (however much this makes GetExecutingAssembly unreliable), but C# solved it, so why can't F#? But IIRC, there's an effort underway in .NET to use caller identity: https://github.com/dotnet/csharplang/issues/4984, which would allow for a reliable workaround. It may not happen before .NET 8 though.

[abelbraaksma]

Even if we wanted to fix this behavior, not having reqsecobj in reference assemblies makes this next to impossible or really awkward (https://github.com/dotnet/fsharp/issues/9283#issuecomment-684998328).

It is sad, but looks like you are right in that there's nothing we can do here...

[ForNeVeR]

C#, notably, "solved" it by not having inlining at all.