Skip to content

ILLink: Stop giving special treatment to icalls #113704

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

ILLink: Stop giving special treatment to icalls #113704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2d3ab7
ILLink: Stop giving special treatment to icalls
mrvoorhe Mar 19, 2025
e09d766
Add missing dependencies in corelib binder
jkotas Mar 20, 2025
dcf9846
Attempted fixes
mrvoorhe Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/coreclr/tools/aot/ILCompiler/ILCompiler_inbuild.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<RuntimeIdentifier>$(NETCoreSdkRuntimeIdentifier)</RuntimeIdentifier>
<PublishDir>$(RuntimeBinDir)$(BuildArchitecture)/ilc/</PublishDir>
<SelfContained>true</SelfContained>
<PublishTrimmed>true</PublishTrimmed>
<!-- Re-enable once we pickup toolchain with https://github.com/dotnet/runtime/pull/113704 -->
<!-- <PublishTrimmed>true</PublishTrimmed> -->
<!--
Publish with AOT on Windows as Single file doesn't work well when an app has to include DiaSymReader.Native.
Publish single-file elsewhere so we don't need to adjust our "AOT in build" logic to handle targeting the SDK RID.
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/tools/aot/crossgen2/crossgen2_inbuild.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<RuntimeIdentifier>$(NETCoreSdkRuntimeIdentifier)</RuntimeIdentifier>
<PublishDir>$(RuntimeBinDir)$(BuildArchitecture)/crossgen2/</PublishDir>
<SelfContained>true</SelfContained>
<PublishTrimmed>true</PublishTrimmed>
<!-- Re-enable once we pickup toolchain with https://github.com/dotnet/runtime/pull/113704 -->
<!-- <PublishTrimmed>true</PublishTrimmed> -->
<!--
Publish with AOT on Windows as Single file doesn't work well when an app has to include DiaSymReader.Native.
Publish single-file elsewhere so we don't need to adjust our "AOT in build" logic to handle targeting the SDK RID.
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/vm/corelib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,9 @@ DEFINE_FIELD_U(m_value, ReflectMethodObject, m_pMD)
DEFINE_CLASS(STUBMETHODINFO, System, RuntimeMethodInfoStub)
DEFINE_FIELD(STUBMETHODINFO, HANDLE, m_value)
DEFINE_METHOD(STUBMETHODINFO, FROMPTR, FromPtr, SM_IntPtr_RetObj)
#ifdef FOR_ILLINK
DEFINE_METHOD(STUBMETHODINFO, CTOR, .ctor, IM_RetVoid)
#endif // FOR_ILLINK

DEFINE_CLASS(CONSTRUCTOR_INFO, Reflection, ConstructorInfo)

Expand Down Expand Up @@ -548,6 +551,9 @@ DEFINE_FIELD_U(m_runtimeType, ReflectModuleBaseObject, m_runtim
DEFINE_FIELD_U(m_runtimeAssembly, ReflectModuleBaseObject, m_runtimeAssembly)
DEFINE_FIELD_U(m_pData, ReflectModuleBaseObject, m_pData)
DEFINE_CLASS(MODULE, Reflection, RuntimeModule)
#ifdef FOR_ILLINK
DEFINE_METHOD(MODULE, CTOR, .ctor, IM_RetVoid)
#endif // FOR_ILLINK

DEFINE_CLASS(TYPE_BUILDER, ReflectionEmit, TypeBuilder)
DEFINE_CLASS(ENUM_BUILDER, ReflectionEmit, EnumBuilder)
Expand Down Expand Up @@ -610,6 +616,9 @@ DEFINE_CLASS(PARAMETER, Reflection, ParameterInfo)
DEFINE_CLASS(PARAMETER_MODIFIER, Reflection, ParameterModifier)

DEFINE_CLASS(POINTER, Reflection, Pointer)
#ifdef FOR_ILLINK
DEFINE_METHOD(POINTER, CTOR, .ctor, IM_RetVoid)
#endif // FOR_ILLINK

DEFINE_CLASS_U(Reflection, Pointer, ReflectionPointer)
DEFINE_FIELD_U(_ptr, ReflectionPointer, _ptr)
Expand Down
4 changes: 0 additions & 4 deletions src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeEnumBuilder.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ public override Type GetEnumUnderlyingType()
return _underlyingType;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2110:ReflectionToDynamicallyAccessedMembers",
Justification = "For instance members with MethodImplOptions.InternalCall, the linker preserves all fields of the declaring type. " +
"The _tb field has DynamicallyAccessedMembersAttribute requirements, but the field access is safe because " +
"Reflection.Emit is not subject to trimming.")]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
private extern void setup_enum_type(Type t);

Expand Down
4 changes: 0 additions & 4 deletions src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeTypeBuilder.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,6 @@ protected override ConstructorBuilder DefineTypeInitializerCore()
null);
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2110:ReflectionToDynamicallyAccessedMembers",
Justification = "For instance member internal calls, the linker preserves all fields of the declaring type. " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the linker know that the Mono unmanaged runtime depends on the RuntimeTypeBuilder fields? Do we need to declare the dependency somewhere?

Copy link
Member

@sbomer sbomer Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place I'm aware of for mono is via a descriptor (https://github.com/dotnet/runtime/blob/main/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml).

I see RuntimeTypeBuilder loaded here:

runtime/src/mono/mono/metadata/reflection.c

Line 3207 in de72cb8

method = mono_class_get_method_from_name_checked (mono_class_get_type_builder_class (), "IsAssignableToInternal", 1, 0, error);

and it looks like the ctor annotations have been set up to preserve fields too:

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeTypeBuilder.Mono.cs

Lines 93 to 113 in de72cb8

[DynamicDependency(nameof(state))] // Automatically keeps all previous fields too due to StructLayout
[DynamicDependency(nameof(IsAssignableToInternal))] // Used from reflection.c: mono_reflection_call_is_assignable_to
internal RuntimeTypeBuilder(RuntimeModuleBuilder mb, TypeAttributes attr, int table_idx, bool is_hidden_global_type = false)
{
this.is_hidden_global_type = is_hidden_global_type;
this.parent = null;
this.attrs = attr;
this.class_size = UnspecifiedTypeSize;
this.table_idx = table_idx;
this.tname = table_idx == 1 ? "<Module>" : "type_" + table_idx.ToString();
this.nspace = string.Empty;
this.fullname = TypeIdentifiers.WithoutEscape(this.tname);
pmodule = mb;
}
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2074:UnrecognizedReflectionPattern",
Justification = "Linker doesn't analyze ResolveUserType but it's an identity function")]
[DynamicDependency(nameof(state))] // Automatically keeps all previous fields too due to StructLayout
[DynamicDependency(nameof(IsAssignableToInternal))] // Used from reflection.c: mono_reflection_call_is_assignable_to
internal RuntimeTypeBuilder(RuntimeModuleBuilder mb, string name, TypeAttributes attr, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent, Type[]? interfaces, PackingSize packing_size, int type_size, Type? nesting_type)

So I think it would make sense to add an entry for these ctors to the descriptors.

"The parent and created fields have DynamicallyAccessedMembersAttribute requirements, but creating the runtime class is safe " +
"because the annotations fully preserve the parent type, and the type created via Reflection.Emit is not subject to trimming.")]
[return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
[MethodImplAttribute(MethodImplOptions.InternalCall)]
private extern TypeInfo create_runtime_class();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3149,7 +3149,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
MarkCustomAttributes (method.MethodReturnType, new DependencyInfo (DependencyKind.ReturnTypeAttribute, method), methodOrigin);
MarkMarshalSpec (method.MethodReturnType, new DependencyInfo (DependencyKind.ReturnTypeMarshalSpec, method), methodOrigin);

if (method.IsPInvokeImpl || method.IsInternalCall) {
if (method.IsPInvokeImpl) {
ProcessInteropMethod (method, methodOrigin);
}

Expand Down
32 changes: 1 addition & 31 deletions ...tools/illink/test/ILLink.RoslynAnalyzer.Tests/Interop/InternalCalls/InternalCallsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,7 @@ public sealed class InternalCallsTests : LinkerTestBase
protected override string TestSuiteName => "Interop/InternalCalls";

[Fact]
public Task UnusedDefaultConstructorIsRemoved ()
{
return RunTest ();
}

[Fact]
public Task UnusedFieldsOfTypesAreNotRemoved ()
{
return RunTest ();
}

[Fact]
public Task UnusedFieldsOfTypesWhenHasThisAreNotRemoved ()
{
return RunTest ();
}

[Fact]
public Task DefaultConstructorOfReturnTypeIsNotRemoved ()
{
return RunTest ();
}

[Fact]
public Task UnusedDefaultConstructorOfTypePassedByRefIsNotRemoved ()
{
return RunTest ();
}

[Fact]
public Task UnusedFieldsOfTypesPassedByRefAreNotRemoved ()
public Task NoSpecialMarking ()
{
return RunTest ();
}
Expand Down
25 changes: 0 additions & 25 deletions ...no.Linker.Tests.Cases/Interop/InternalCalls/DefaultConstructorOfReturnTypeIsNotRemoved.cs
View file
Open in desktop

This file was deleted.

36 changes: 36 additions & 0 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/Interop/InternalCalls/NoSpecialMarking.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Runtime.CompilerServices;
using Mono.Linker.Tests.Cases.Expectations.Assertions;

namespace Mono.Linker.Tests.Cases.Interop.InternalCalls;

public class NoSpecialMarking
{
public static void Main ()
{
A a = null;
SomeMethod (ref a, null, null);
}

[Kept]
class A;

[Kept]
class B;

[Kept]
class C;

[Kept]
class D
{
int field1;
int field2;
}

[Kept]
[MethodImpl (MethodImplOptions.InternalCall)]
static extern C SomeMethod (ref A a, B b, D d);
}
30 changes: 0 additions & 30 deletions ...k/test/Mono.Linker.Tests.Cases/Interop/InternalCalls/UnusedDefaultConstructorIsRemoved.cs
View file
Open in desktop

This file was deleted.

31 changes: 0 additions & 31 deletions ...ests.Cases/Interop/InternalCalls/UnusedDefaultConstructorOfTypePassedByRefIsNotRemoved.cs
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions ...nk/test/Mono.Linker.Tests.Cases/Interop/InternalCalls/UnusedFieldsOfTypesAreNotRemoved.cs
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions ...o.Linker.Tests.Cases/Interop/InternalCalls/UnusedFieldsOfTypesPassedByRefAreNotRemoved.cs
View file
Open in desktop

This file was deleted.

24 changes: 0 additions & 24 deletions ...o.Linker.Tests.Cases/Interop/InternalCalls/UnusedFieldsOfTypesWhenHasThisAreNotRemoved.cs
View file
Open in desktop

This file was deleted.

Loading