Skip to content

Commit 7d8d8f0

Browse files
mrvoorhemrvoorhe
committed
ILLink : Fix instantiation tracking issue with icall/pinvoke parameters
Along the lines of #113434. The existing `ProcessInteropMethod` has a bug where the return type and/or out parameter types will not be marked as instantiated. In this case, the bug happens when the return type or parameter types do not have a `.ctor()`. This is because `ProcessInteropMethod` will call `MarkDefaultConstructor` on the type which will cause the type to be marked instantiated if the `.ctor()` exists.
1 parent e9303d3 commit 7d8d8f0

File tree

3 files changed

+185
-0
lines changed

3 files changed

+185
-0
lines changed

‎src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3369,6 +3369,11 @@ void ProcessInteropMethod (MethodDefinition method, MessageOrigin origin)
33693369
if (!returnTypeDefinition.IsImport) {
33703370
// What we keep here is correct most of the time, but not every time. Fine for now.
33713371
MarkDefaultConstructor (returnTypeDefinition, new DependencyInfo (DependencyKind.InteropMethodDependency, method), origin);
3372+
3373+
// We need to make sure the return type is marked as instantiated. If the type happens to have a .ctor() then we'll get lucky and that
3374+
// will trigger marking as instantiated. We need to also cover the case when a type doesn't have a .ctor()
3375+
MarkRequirementsForInstantiatedTypes (returnTypeDefinition);
3376+
33723377
MarkFields (returnTypeDefinition, includeStaticFields, new DependencyInfo (DependencyKind.InteropMethodDependency, method), origin);
33733378
}
33743379
}
@@ -3391,6 +3396,10 @@ void ProcessInteropMethod (MethodDefinition method, MessageOrigin origin)
33913396
MarkFields (paramTypeDefinition, includeStaticFields, new DependencyInfo (DependencyKind.InteropMethodDependency, method), origin);
33923397
if (pd.ParameterType.IsByReference) {
33933398
MarkDefaultConstructor (paramTypeDefinition, new DependencyInfo (DependencyKind.InteropMethodDependency, method), origin);
3399+
3400+
// We need to make sure the return type is marked as instantiated. If the type happens to have a .ctor() then we'll get lucky and that
3401+
// will trigger marking as instantiated. We need to also cover the case when a type doesn't have a .ctor()
3402+
MarkRequirementsForInstantiatedTypes (paramTypeDefinition);
33943403
}
33953404
}
33963405
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Interop/InternalCalls/OutTypesAreMarkedInstantiated.cs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Runtime.CompilerServices;
5+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
6+
7+
namespace Mono.Linker.Tests.Cases.Interop.InternalCalls;
8+
9+
public class OutTypesAreMarkedInstantiated
10+
{
11+
public static void Main ()
12+
{
13+
FooRefParameter refParmaeter = null;
14+
InteropMethod (null, ref refParmaeter, out FooOutParameter outParameter);
15+
UsedToMarkMethods (null, null, null, null);
16+
}
17+
18+
[Kept]
19+
[MethodImpl (MethodImplOptions.InternalCall)]
20+
static extern FooReturn InteropMethod (FooNormalParameter normal, ref FooRefParameter @ref, out FooOutParameter @out);
21+
22+
[Kept]
23+
static void UsedToMarkMethods (FooReturn f, FooNormalParameter n, FooRefParameter r, FooOutParameter o)
24+
{
25+
f.Method ();
26+
n.Method ();
27+
r.Method ();
28+
o.Method ();
29+
}
30+
}
31+
32+
33+
public class FooReturn
34+
{
35+
// Define a non-default constructor. This is required in order to trigger the bug
36+
public FooReturn (int a)
37+
{
38+
}
39+
40+
// This method should not have it's body modified because the linker should consider it as instantiated
41+
[Kept]
42+
public void Method ()
43+
{
44+
throw new System.NotImplementedException ();
45+
}
46+
}
47+
48+
public class FooOutParameter
49+
{
50+
// Define a non-default constructor. This is required in order to trigger the bug
51+
public FooOutParameter (int a)
52+
{
53+
}
54+
55+
// This method should not have it's body modified because the linker should consider it as instantiated
56+
[Kept]
57+
public void Method ()
58+
{
59+
throw new System.NotImplementedException ();
60+
}
61+
}
62+
63+
public class FooRefParameter
64+
{
65+
// Define a non-default constructor. This is required in order to trigger the bug
66+
public FooRefParameter (int a)
67+
{
68+
}
69+
70+
// This method should not have it's body modified because the linker should consider it as instantiated
71+
[Kept]
72+
public void Method ()
73+
{
74+
throw new System.NotImplementedException ();
75+
}
76+
}
77+
78+
public class FooNormalParameter
79+
{
80+
[Kept]
81+
// This parameter will not be marked as instantiated because it is not an out or ref parameter. This will lead to the linker applying the
82+
// unreachable bodies optimization
83+
[ExpectBodyModified]
84+
public void Method ()
85+
{
86+
throw new System.NotImplementedException ();
87+
}
88+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Interop/PInvoke/OutTypesAreMarkedInstantiated.cs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Runtime.InteropServices;
5+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
6+
7+
namespace Mono.Linker.Tests.Cases.Interop.PInvoke;
8+
9+
[KeptModuleReference ("Unused")]
10+
public class OutTypesAreMarkedInstantiated
11+
{
12+
public static void Main ()
13+
{
14+
FooRefParameter refParmaeter = null;
15+
InteropMethod (null, ref refParmaeter, out FooOutParameter outParameter);
16+
UsedToMarkMethods (null, null, null, null);
17+
}
18+
19+
[Kept]
20+
[DllImport ("Unused")]
21+
static extern FooReturn InteropMethod (FooNormalParameter normal, ref FooRefParameter @ref, out FooOutParameter @out);
22+
23+
[Kept]
24+
static void UsedToMarkMethods (FooReturn f, FooNormalParameter n, FooRefParameter r, FooOutParameter o)
25+
{
26+
f.Method ();
27+
n.Method ();
28+
r.Method ();
29+
o.Method ();
30+
}
31+
}
32+
33+
public class FooReturn
34+
{
35+
// Define a non-default constructor. This is required in order to trigger the bug
36+
public FooReturn (int a)
37+
{
38+
}
39+
40+
// This method should not have it's body modified because the linker should consider it as instantiated
41+
[Kept]
42+
public void Method ()
43+
{
44+
throw new System.NotImplementedException ();
45+
}
46+
}
47+
48+
public class FooOutParameter
49+
{
50+
// Define a non-default constructor. This is required in order to trigger the bug
51+
public FooOutParameter (int a)
52+
{
53+
}
54+
55+
// This method should not have it's body modified because the linker should consider it as instantiated
56+
[Kept]
57+
public void Method ()
58+
{
59+
throw new System.NotImplementedException ();
60+
}
61+
}
62+
63+
public class FooRefParameter
64+
{
65+
// Define a non-default constructor. This is required in order to trigger the bug
66+
public FooRefParameter (int a)
67+
{
68+
}
69+
70+
// This method should not have it's body modified because the linker should consider it as instantiated
71+
[Kept]
72+
public void Method ()
73+
{
74+
throw new System.NotImplementedException ();
75+
}
76+
}
77+
78+
public class FooNormalParameter
79+
{
80+
[Kept]
81+
// This parameter will not be marked as instantiated because it is not an out or ref parameter. This will lead to the linker applying the
82+
// unreachable bodies optimization
83+
[ExpectBodyModified]
84+
public void Method ()
85+
{
86+
throw new System.NotImplementedException ();
87+
}
88+
}

0 commit comments

Comments
 (0)