[NativeAOT] Assert when compiling System.Runtime.Tests

[VSadov]

The assert happens with current bits from main.
I hoped that it could be somehow caused by the barrier issue that was fixed in #72919.

However the assert reproduces after the fix has been merged and appears to be deterministic.

It looks like it is happening in the constructor of DataflowAnalyzedMethodNode

        public DataflowAnalyzedMethodNode(MethodIL methodIL)
        {
            Debug.Assert(methodIL.OwningMethod.IsTypicalMethodDefinition);
here ==>    Debug.Assert(!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember(methodIL.OwningMethod));
            _methodIL = methodIL;
        }

  Process terminated. Assertion failed.
     at ILCompiler.DependencyAnalysis.DataflowAnalyzedMethodNode..ctor(MethodIL methodIL) in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs:line 27
     at ILCompiler.DependencyAnalysis.NodeFactory.<>c.<CreateNodeCaches>b__39_33(MethodILKey il) in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs:line 386
     at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
     at ILCompiler.UsageBasedMetadataManager.AddDataflowDependency(DependencyList& dependencies, NodeFactory factory, MethodIL methodIL, String reason) in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs:line 931
     at Internal.IL.ILImporter.ImportCall(ILOpcode opcode, Int32 token) in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs:line 264
     at Internal.IL.ILImporter.ImportBasicBlock(BasicBlock basicBlock) in /home/robox/vsadov/runtime01/src/coreclr/tools/Common/TypeSystem/IL/ILImporter.cs:line 427
     at Internal.IL.ILImporter.ImportBasicBlocks() in /home/robox/vsadov/runtime01/src/coreclr/tools/Common/TypeSystem/IL/ILImporter.cs:line 311
     at Internal.IL.ILImporter.Import() in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs:line 156
     at ILCompiler.ILScanner.CompileSingleMethod(ScannedMethodNode methodCodeNodeNeedingCode) in /home/robox/vsadov/runtime01/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs:line 123
     at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`1.<ForWorker>b__1(RangeWorker& currentWorker, Int32 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
     at System.Threading.Tasks.TaskReplicator.Replica.Execute()
     at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
     at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
     at System.Threading.ThreadPoolWorkQueue.Dispatch()
     at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
  Aborted

To repro, compile runtime and tests on a linux-arm64 machine like:

./build.sh -arch arm64 -os Linux -s clr.alljits+clr.tools+clr.nativeaotlibs+clr.nativeaotruntime+libs+libs.tests -rc Checked -lc Release /p:TestNativeAot=true -test -maxcpucount:10

The failure is not specific to linux-arm64

win-x64 repro:

build.cmd -s clr.alljits+clr.tools+clr.nativeaotlibs+clr.nativeaotruntime+libs+libs.tests -rc Checked -lc Release /p:TestNativeAot=true -test --maxcpucount:16

[VSadov]

CC: @MichalStrehovsky - this is with current bits in main.
Not sure yet if this is specific to linux-arm64 or reproduces on other configs as well.

[MichalStrehovsky]

The logic is new (#71485) - I don't think it's severe - I think you can just comment it out locally to unblock yourself.

Cc @vitek-karas

[VSadov]

It happens on win-x64 as well

[VSadov]

There is a similar assert in InterproceduralScan

        // Scans the method as well as any nested functions (local functions or lambdas) and state machines
        // reachable from it.
        public virtual void InterproceduralScan(MethodIL startingMethodBody)
        {
            MethodDesc startingMethod = startingMethodBody.OwningMethod;
            Debug.Assert(startingMethod.IsTypicalMethodDefinition);

            // We should never have created a DataFlowAnalyzedMethodNode for compiler generated methods
            // since their data flow analysis is handled as part of their parent method analysis.
            Debug.Assert(!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember(startingMethod));

I am guessing if I comment one, I will start hitting the other.

[VSadov]

after commenting the assert in both places tests can build and pass.

[vitek-karas]

Thanks for finding this Vlad.

The short version is that we haven't ported all of the functionality from linker yet - in this case specifically the "Reflection access to compiler generated methods".

The situation of this repro:

• Assembly which is fully preserved via a descriptor
• User method which looks something like this:

void UserMethod()
{
    if (AlwaysFalse)  // Or a feature switch which will evaluate to constant false
    {
        LocalFunction();
    }

    void LocalFunction() { do something which requires data flow }
}

• In this case the compiler generated method (the LocalFunction) will be "marked" (both by linker and AOT)
• But AOT will try to do data flow on it
  • This is because it doesn't have an associated user method - the code to find that doesn't see the call to it since it has been removed by branch removal (constant prop)
  • This leads to the asserts, since we never expect to do data flow on compiler generated method directly (only indirectly through its user method)
• Linker has a guard against this which we haven't ported yet - basically this line: https://github.com/dotnet/linker/blob/65e21d78731ae5825b3921a870221fbc8fdeee67/src/linker/Linker.Steps/MarkStep.cs#L3425

I'll look into the fix some more - there's also an interesting question regarding warning behavior in this case (both for linker and AOT).

/cc @sbomer