Framework trimming inlines CoreCLR's CoreLib into other inbox libraries

[VSadov]

I will disable System.Data.DataSetExtensions.Tests for now to unblock CI, but perhaps the tests can be fixed

---- System.NotSupportedException : 'System.Data.DataRowExtensions+UnboxT1[System.Nullable1[System.Int32]].NullableFieldSystem.Int32' is missing native code.

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I will disable System.Data.DataSetExtensions.Tests for now to unblock CI, but perhaps the tests can be fixed

Author:	VSadov
Assignees:	-
Labels:	untriaged, area-NativeAOT-coreclr
Milestone:	-

[VSadov]

Error message
System.TypeInitializationException : A type initializer threw an exception. To determine which type, inspect the InnerException's StackTrace property.
---- System.NotSupportedException : 'System.Data.DataRowExtensions+UnboxT1[System.Nullable1[System.Int32]].NullableFieldSystem.Int32' is missing native code. MethodInfo.MakeGenericMethod() is not compatible with AOT compilation. Inspect and fix AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility

Stack trace
at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x163
at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnGCStaticBase(StaticClassConstructionContext*, Object) + 0xd
at System.Data.DataRowExtensions.Field[T](DataRow, String) + 0x16
at MonoTests.System.Data.DataRowExtensionsTest.Field_T_DBNullFieldValue() + 0x83
at System.Data.DataSetExtensions.Tests!+0x809144
at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) + 0xc4
----- Inner Stack Trace -----
at System.Reflection.Runtime.MethodInfos.RuntimeNamedMethodInfo1.GetUncachedMethodInvoker(RuntimeTypeInfo[], MemberInfo) + 0x2f at System.Reflection.Runtime.MethodInfos.RuntimeNamedMethodInfo1.MakeGenericMethod(Type[]) + 0x18e
at System.Data.DataRowExtensions.UnboxT1.<Create>g__CreateWhenDynamicCodeSupported|1_0() + 0x6d at System.Data.DataRowExtensions.UnboxT1.Create() + 0x9
at System.Data.DataRowExtensions.UnboxT`1..cctor() + 0x1e
at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0xc4

[MichalStrehovsky]

DataRowExtensions was fixed up to be AOT safe so this is very suspicious:

runtime/src/libraries/System.Data.Common/src/System/Data/DataRowExtensions.cs

Lines 149 to 156 in cfdf0f6

	if (typeof(T).IsValueType && default(T) == null)
	{
	if (!RuntimeFeature.IsDynamicCodeSupported)
	return NullableFieldUsingReflection;
	return CreateWhenDynamicCodeSupported();
	}
	return NonNullableField;

We were not even supposed to compile CreateWhenDynamicCodeSupported.

[MichalStrehovsky]

Ok, this is a pretty bad product issue that affects more than just NativeAOT. I guess it's a happy accident we found it. The thing that surfaced it was 5435aa8 that changed how we run NativeAOT testing - we started building all of CoreCLR. I couldn't repro the issue locally because I don't like building junk I don't need - building all of CoreCLR is what triggers the problem though.

runtime/eng/Subsets.props

Line 106 in 7e10f81

<UseNativeAotCoreLib Condition="$(_subset.Contains('+clr.nativeaotlibs+')) and !$(_subset.Contains('+clr.native+')) and !$(_subset.Contains('+clr.runtime+'))">true</UseNativeAotCoreLib>

If we build clr.runtime subset, we no longer build libraries against NativeAOT's corelib, but use CoreCLR's corelib instead. This is not desirable - we want to use NativeAOT's so that we run APICompat with it. Another thing this changes is what corelib we use when trimming the framework as part of framework build. Guess what RuntimeFeature.IsDynamicCodeSupported is on CoreCLR's corelib. The illink that we use during library build turns the above snippet into:

        if (typeof(T).get_IsValueType() && default(T) == null)
	{
		if (1 == 0)
		{
		}
		return CreateWhenDynamicCodeSupported();
	}

We could make the test pass again by switching back to using NativeAOT's corelib during libraries build.

But here's why it's a product issue: all shipping libraries come from a build that uses CoreCLR corelib. I think this also includes platforms targeted only by Mono - e.g. the System.Data.Common in this test is not RID-specific and we probably don't have an e.g. iOS specific build.

The worst part of this: we don't detect any of this in our testing because the testing always uses the "expected" corelib (Mono's on Mono platforms, NativeAOT's in NativeAOT testing).

Basically, framework build is inlining arbitrary parts of CoreLib, but we don't know whether the implementations couldn't actually differ.

Cc @dotnet/ilc-contrib @vitek-karas @sbomer

[jkotas]

cc @ViktorHofer

[jkotas]

This sounds like we run runtime-pack trimming during runtime-neutral part of the libraries build that is not correct. Instead, it needs to run during runtime-specific part of the build when we are building runtime packs (similar to where R2R compilation runs for CoreCLR).

[ViktorHofer]

But here's why it's a product issue: all shipping libraries come from a build that uses CoreCLR corelib. I think this also includes platforms targeted only by Mono - e.g. the System.Data.Common in this test is not RID-specific and we probably don't have an e.g. iOS specific build.

All shipping libraries (packages, etc.) build against CoreLib's reference assembly which is expected to be runtime neutral. cc @akoeplinger. How does the runtime variant of CoreLib come into play here?

Another thing this changes is what corelib we use when trimming the framework as part of framework build. Guess what RuntimeFeature.IsDynamicCodeSupported is on CoreCLR's corelib. The illink that we use during library build turns the above snippet into:

When you say "framework build", I assume you refer to the shared framework packages build (which lives in src/installer/pkg/sfx)?

This sounds like we run runtime-pack trimming during runtime-neutral part of the libraries build that is not correct.

AFAIK the libraries build doesn't do any kind of runtime-pack trimming, neither do we invoke crossgen2. The only trimming that happens is inside illink.targets that runs as part of a library's individual build.

[MichalStrehovsky]

All shipping libraries (packages, etc.) build against CoreLib's reference assembly which is expected to be runtime neutral. cc @akoeplinger. How does the runtime variant of CoreLib come into play here?

When we run trimming on the framework, we use the implementation assembly. The ref assembly wouldn't work for trimming.

[ViktorHofer]

That's understood. But as I said, AFAIK we don't do any runtime-pack trimming during the libraries build. I.e. our out-of-band libraries packages like System.Formats.Cbor that don't ship inside the shared framework don't get trimmed in any special way aside from what is happening in illink.targets. And correct me if I'm wrong, the library-mode trimming in illink.targets doesn't receive CoreLib as an input.

[jkotas]

we don't do any runtime-pack trimming during the libraries build.

The targets for runtime-pack trimming are under src/libraries:

runtime/src/libraries/sfx.proj

Lines 41 to 47 in 2a31a5b

	<Target Name="TrimSharedFrameworkAssemblies"
	AfterTargets="Build"
	DependsOnTargets="ResolveProjectReferences;GetTrimSharedFrameworkAssembliesInputs;PrepareForAssembliesTrim"
	Condition="'$(RefOnly)' != 'true'"
	Inputs="@(SharedFrameworkAssembly);@(SharedFrameworkSuppressionsXml);$(ILLinkTasksAssembly)"
	Outputs="$(SharedFrameworkAssembliesMarkerFile)">
	<Message Text="$(MSBuildProjectName) -> Trimming $(OutputRid) shared framework assemblies with ILLinker..." Importance="high" />

I took a closer look. I do not think we have a product issue for native AOT here (I have not checked Mono). The native ILCompiler package gets copy of assemblies before runtime-pack trimming. For example, System.Data.Common assembly that ends up in ILCompiler package comes from artifacts\obj\System.Data.Common\Release\net8.0\System.Data.Common.dll. The copy from trimmed runtime pack is at artifacts\bin\ILLinkTrimAssembly\net8.0-windows-Release-x64\trimmed-runtimepack\System.Data.Common.dll and it is not used for ILCompiler package. @MichalStrehovsky Do you agree that there is no product issue here? If you agree, this is only a problem with test infrastructure composing the product in its own way.

[ViktorHofer]

Yes, what you link to is just a validation step that doesn't impact the build outputs. That's why I didn't mention it. We have the same step in oob.proj as well.