[Mono][full-aot] Fix runtime invoke with delegate parameter #95824

fanyang-mono · 2023-12-09T14:34:56Z

A follow up fix for the runtime invoke issue found in #95469

TLDR: This bug only affected iOS target in release build. The following library test needs to run on iOS or full-aot on arm64 platform to lockdown this behavior:

runtime/src/libraries/System.Runtime/tests/System.Reflection.Tests/MethodCommonTests.cs

Line 280 in 701428f

public unsafe void TestFunctionPointerAsArgType()

This issue is only reproducible when dyn_call_info field is set. dyn_call_info field is only set when condition !mono_llvm_only && (mono_aot_only || mini_debug_options.dyn_runtime_invoke) is met and MONO_ARCH_DYN_CALL_SUPPORTED is set.

MONO_ARCH_DYN_CALL_SUPPORTED is only set for
- amd64, arm64 and amd platforms
Only the following modes have mono_llvm_only setting to false, and mono_aot_only setting to true
- MONO_AOT_MODE_FULL
  - This mode is set when using --full-aot switch
- MONO_AOT_MODE_INTERP
  - This mode is only set when AOT is not enabled for WASM and WASI.

fanyang-mono · 2023-12-09T15:22:04Z

Currently, only these library tests are run on iOS devices. I would like to add System.Reflection.Tests to the list to lock down the behavior of this issue. Do you happen to know where that logic lives? @kotlarmilos @steveisok

fanyang-mono · 2023-12-09T15:22:40Z

/azp run runtime-ioslike

azure-pipelines · 2023-12-09T15:22:48Z

Azure Pipelines successfully started running 1 pipeline(s).

kotlarmilos · 2023-12-11T09:50:45Z

Currently, only these library tests are run on iOS devices. I would like to add System.Reflection.Tests to the list to lock down the behavior of this issue. Do you happen to know where that logic lives? @kotlarmilos @steveisok

The library tests for iOS are listed here

runtime/src/libraries/tests.proj

Lines 651 to 671 in c28bec4

    
           <ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetOS)' == 'ios' and '$(RunAOTCompilation)' == 'true'"> 
        
             <!-- Only System.Runtime tests on iOS for now --> 
        
             <ProjectReference Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Runtime.Tests\System.Runtime.Tests.csproj" /> 
        
             <ProjectReference Include="$(RepoRoot)\src\tests\FunctionalTests\iOS\Device\**\*.Test.csproj" 
        
                               Exclude="@(ProjectExclusions)" 
        
                               BuildInParallel="false" /> 
        
           </ItemGroup> 
        
           <ItemGroup Condition="'$(ArchiveTests)' == 'true' and ('$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvos') and '$(UseNativeAOTRuntime)' == 'true'"> 
        
             <ProjectReference Include="$(RepoRoot)\src\tests\FunctionalTests\$(TargetOS)\Device\**\*.Test.csproj" 
        
                             Exclude="@(ProjectExclusions)" 
        
                             BuildInParallel="false" /> 
        
           </ItemGroup> 
        
           <ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetOS)' == 'ios' and '$(IsManualOrRollingBuild)' == 'true'"> 
        
             <!-- These crash on tvOS, but do not on iOS. Run these only on the rolling builds --> 
        
             <ProjectReference Include="$(MSBuildThisFileDirectory)System.IO.MemoryMappedFiles\tests\System.IO.MemoryMappedFiles.Tests.csproj" /> 
        
             <ProjectReference Include="$(MSBuildThisFileDirectory)System.Runtime.Numerics\tests\System.Runtime.Numerics.Tests.csproj" /> 
        
             <ProjectReference Include="$(MSBuildThisFileDirectory)System.IO.Hashing\tests\System.IO.Hashing.Tests.csproj" /> 
        
           </ItemGroup>

Currently, we are facing throughput issues due to the number of devices in the testing pool. As a result, only a few tests are executed on iOS, while the remaining tests are run on tvOS. We consider these two platforms to be interchangeable in most cases. If the System.Reflection.Tests are running on tvOS, we should consider them as covered.

fanyang-mono · 2023-12-11T14:53:57Z

Currently, only these library tests are run on iOS devices. I would like to add System.Reflection.Tests to the list to lock down the behavior of this issue. Do you happen to know where that logic lives? @kotlarmilos @steveisok

The library tests for iOS are listed here

runtime/src/libraries/tests.proj

Lines 651 to 671 in c28bec4

<ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetOS)' == 'ios' and '$(RunAOTCompilation)' == 'true'">



<ProjectReference Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Runtime.Tests\System.Runtime.Tests.csproj" />

<ProjectReference Include="$(RepoRoot)\src\tests\FunctionalTests\iOS\Device\**\*.Test.csproj"

Exclude="@(ProjectExclusions)"

BuildInParallel="false" />

</ItemGroup>

<ItemGroup Condition="'$(ArchiveTests)' == 'true' and ('$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvos') and '$(UseNativeAOTRuntime)' == 'true'">

<ProjectReference Include="$(RepoRoot)\src\tests\FunctionalTests\$(TargetOS)\Device\**\*.Test.csproj"

Exclude="@(ProjectExclusions)"

BuildInParallel="false" />

</ItemGroup>

<ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetOS)' == 'ios' and '$(IsManualOrRollingBuild)' == 'true'">



<ProjectReference Include="$(MSBuildThisFileDirectory)System.IO.MemoryMappedFiles\tests\System.IO.MemoryMappedFiles.Tests.csproj" />

<ProjectReference Include="$(MSBuildThisFileDirectory)System.Runtime.Numerics\tests\System.Runtime.Numerics.Tests.csproj" />

<ProjectReference Include="$(MSBuildThisFileDirectory)System.IO.Hashing\tests\System.IO.Hashing.Tests.csproj" />

</ItemGroup>

Currently, we are facing throughput issues due to the number of devices in the testing pool. As a result, only a few tests are executed on iOS, while the remaining tests are run on tvOS. We consider these two platforms to be interchangeable in most cases. If the System.Reflection.Tests are running on tvOS, we should consider them as covered.

Then I would expect the test mentioned in the description area failing on main.

fanyang-mono · 2023-12-11T15:32:06Z

This is the log of nativeaot/SmokeTests/TrimmingBehaviors/TrimmingBehaviors/TrimmingBehaviors.sh. The test passed.

[10:54:46.5220840] 2023-12-09 07:54:46.584 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Running test Dataflow.Run =====
[10:54:46.5692380] 2023-12-09 07:54:46.622 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Test Dataflow.Run succeeded =====
[10:54:46.5692590] 2023-12-09 07:54:46.622 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Running test DeadCodeElimination.Run =====
[10:54:46.5692710] 2023-12-09 07:54:46.623 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing instance methods on unallocated types
[10:54:46.5692750] 2023-12-09 07:54:46.624 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing virtual methods on never derived abstract types
[10:54:46.5692790] 2023-12-09 07:54:46.624 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing abstract classes that might have methods reachable through devirtualization
[10:54:46.5692830] 2023-12-09 07:54:46.624 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing more abstract classes that might have methods reachable through devirtualization
[10:54:46.5692870] 2023-12-09 07:54:46.634 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing array element type optimizations
[10:54:46.5693080] 2023-12-09 07:54:46.634 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing unused static virtual method optimization
[10:54:46.5693130] 2023-12-09 07:54:46.635 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] False
[10:54:46.5693170] 2023-12-09 07:54:46.635 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing dead branches guarded by typeof in generic code removal
[10:54:46.5693210] 2023-12-09 07:54:46.637 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Testing we were able to make non-readonly field read-only: 42
[10:54:46.5693250] 2023-12-09 07:54:46.637 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+CanBeMadeReadOnly
[10:54:46.5693290] 2023-12-09 07:54:46.638 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+IsReflectedOn
[10:54:46.5693330] 2023-12-09 07:54:46.638 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+IsModified
[10:54:46.5693660] 2023-12-09 07:54:46.638 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+CanBeMadeReadOnlyProperty
[10:54:46.5693710] 2023-12-09 07:54:46.638 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] DeadCodeElimination+TestUnmodifiableInstanceFieldOptimization+WithInitOnlyPropertyWrite
[10:54:46.5693750] 2023-12-09 07:54:46.639 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Test DeadCodeElimination.Run succeeded =====
[10:54:46.5693790] 2023-12-09 07:54:46.640 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Running test FeatureSwitches.Run =====
[10:54:46.5693830] 2023-12-09 07:54:46.647 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] Resource list
[10:54:46.5693870] 2023-12-09 07:54:46.650 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] KeptResource
[10:54:46.5693900] 2023-12-09 07:54:46.650 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Test FeatureSwitches.Run succeeded =====
[10:54:46.5693990] 2023-12-09 07:54:46.650 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Running test ILLinkDescriptor.Run =====
[10:54:46.5694070] 2023-12-09 07:54:46.651 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Test ILLinkDescriptor.Run succeeded =====
[10:54:46.5694110] 2023-12-09 07:54:46.651 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Running test DependencyInjectionPattern.Run =====
[10:54:46.5694150] 2023-12-09 07:54:46.652 nativeaot_SmokeTests_TrimmingBehaviors[66280:55356535] ===== Test DependencyInjectionPattern.Run succeeded =====

However, the test was flagged as failed. And the log for that is

      [10:54:47] dbug: Process mlaunch exited with 0
      [10:54:52] dbug: Killing process tree of 24652...
      [10:54:52] dbug: Pids to kill: 24652
      [10:54:52] dbug: Detected exit code 0 from

This seems to be a test infrastructure issue. Opened a github issue #95862.

cc: @kotlarmilos

fanyang-mono · 2023-12-11T15:32:35Z

All other CI failures are known.

fanyang-mono · 2023-12-11T15:38:11Z

This is why the test mentioned in the description box weren't failing on main. They are currently disabled on tvOS because of existing issue. System.Reflection.InvokeInterpreted.Tests.csproj or System.Reflection.Tests.csproj would have exposed this issue on CI

runtime/src/libraries/tests.proj

Lines 262 to 282 in c28bec4

    
             <ItemGroup Condition="'$(TargetOS)' == 'tvos' and '$(RunDisablediOSTests)' != 'true'"> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.Emit\tests\System.Reflection.Emit.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.Emit.Lightweight/tests/System.Reflection.Emit.Lightweight.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.DispatchProxy/tests/System.Reflection.DispatchProxy.Tests.csproj" /> 
        
               <!-- https://github.com/dotnet/runtime/issues/93852 --> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Reflection.Tests/InvokeEmit/System.Reflection.InvokeEmit.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Reflection.Tests/InvokeInterpreted/System.Reflection.InvokeInterpreted.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Threading.Tasks.Parallel/tests/System.Threading.Tasks.Parallel.Tests.csproj" /> 
        
               <!-- Has deps that JIT, need re-done in order to pass --> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj" /> 
        
               <!-- 
        
                   Test suites hang and time out. 
        
                   https://github.com/dotnet/runtime/issues/60713 
        
               --> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.DependencyInjection/tests/DI.External.Tests/Microsoft.Extensions.DependencyInjection.ExternalContainers.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Net.NetworkInformation/tests/FunctionalTests/System.Net.NetworkInformation.Functional.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime/tests/System.Reflection.Tests/System.Reflection.Tests.csproj" /> 
        
               <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Threading.Channels/tests/System.Threading.Channels.Tests.csproj" />

Fix runtime invoke with delegate parameter

44e0a68

fanyang-mono marked this pull request as ready for review December 9, 2023 14:35

fanyang-mono requested a review from vargaz as a code owner December 9, 2023 14:35

ghost assigned fanyang-mono Dec 9, 2023

fanyang-mono requested review from lambdageek and SamMonoRT as code owners December 9, 2023 14:35

dotnet-issue-labeler bot added the area-Codegen-JIT-mono label Dec 9, 2023

fanyang-mono changed the title ~~[Mono] Fix runtime invoke with delegate parameter~~ [Mono][full-aot] Fix runtime invoke with delegate parameter Dec 9, 2023

fanyang-mono added area-Codegen-AOT-mono and removed area-Codegen-JIT-mono labels Dec 9, 2023

vargaz approved these changes Dec 9, 2023

View reviewed changes

fanyang-mono merged commit e7da682 into dotnet:main Dec 11, 2023
114 of 120 checks passed

github-actions bot locked and limited conversation to collaborators Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mono][full-aot] Fix runtime invoke with delegate parameter #95824

[Mono][full-aot] Fix runtime invoke with delegate parameter #95824

fanyang-mono commented Dec 9, 2023 •

edited

Loading

fanyang-mono commented Dec 9, 2023

fanyang-mono commented Dec 9, 2023

azure-pipelines bot commented Dec 9, 2023

kotlarmilos commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023 •

edited

Loading

fanyang-mono commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023

[Mono][full-aot] Fix runtime invoke with delegate parameter #95824

[Mono][full-aot] Fix runtime invoke with delegate parameter #95824

Conversation

fanyang-mono commented Dec 9, 2023 • edited Loading

fanyang-mono commented Dec 9, 2023

fanyang-mono commented Dec 9, 2023

azure-pipelines bot commented Dec 9, 2023

kotlarmilos commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023 • edited Loading

fanyang-mono commented Dec 11, 2023

fanyang-mono commented Dec 11, 2023

fanyang-mono commented Dec 9, 2023 •

edited

Loading

fanyang-mono commented Dec 11, 2023 •

edited

Loading