Skip to content

Commit 9f77f8a

Browse files
siegfriedpammersiegfriedpammer
committed
Prevent inlining of call arguments when doing so would change order of evaluation with regards to the implicit ldobj performed by a constrained.callvirt.
1 parent c6b6c09 commit 9f77f8a

19 files changed

Lines changed: 407 additions & 31 deletions

‎ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
1717
// DEALINGS IN THE SOFTWARE.
1818

19-
using System;
20-
using System.CodeDom.Compiler;
2119
using System.IO;
2220
using System.Linq;
2321
using System.Runtime.CompilerServices;
@@ -313,6 +311,12 @@ public async Task Jmp()
313311
await RunIL("Jmp.il");
314312
}
315313

314+
[Test]
315+
public async Task NonGenericConstrainedCallVirt()
316+
{
317+
await RunIL("NonGenericConstrainedCallVirt.il", CompilerOptions.UseRoslynLatest);
318+
}
319+
316320
[Test]
317321
public async Task StackTests()
318322
{

‎ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
<None Include="TestCases\ILPretty\Issue2260SwitchString.il" />
9696
<None Include="TestCases\ILPretty\Issue3442.il" />
9797
<None Include="TestCases\ILPretty\MonoFixed.il" />
98+
<None Include="TestCases\Correctness\NonGenericConstrainedCallVirt.il" />
9899
<None Include="TestCases\ILPretty\UnknownTypes.cs" />
99100
<None Include="TestCases\ILPretty\UnknownTypes.il" />
100101
<None Include="TestCases\ILPretty\EvalOrder.cs" />

‎ICSharpCode.Decompiler.Tests/TestCases/Correctness/NonGenericConstrainedCallVirt.il‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
.assembly extern mscorlib
2+
{
3+
.publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4..
4+
.ver 4:0:0:0
5+
}
6+
7+
.assembly _
8+
{
9+
.hash algorithm 0x00008004 // SHA1
10+
.ver 0:0:0:0
11+
}
12+
13+
.class public auto ansi beforefieldinit NonGenericConstrainedCallVirt
14+
extends [mscorlib]System.Object
15+
{
16+
// Methods
17+
.method public hidebysig static
18+
void Main () cil managed
19+
{
20+
// Method begins at RVA 0x2050
21+
// Code size 16 (0x10)
22+
.entrypoint
23+
.maxstack 8
24+
25+
IL_0000: ldstr "A"
26+
IL_0005: newobj instance void C::.ctor(string)
27+
IL_000a: call void NonGenericConstrainedCallVirt::Foo(class C)
28+
IL_000f: ret
29+
} // end of method NonGenericConstrainedCallVirt::Main
30+
31+
.method private hidebysig static
32+
void Foo (
33+
class C arg
34+
) cil managed
35+
{
36+
// Method begins at RVA 0x2064
37+
// Code size 25 (0x19)
38+
.maxstack 8
39+
40+
IL_0000: ldarga arg
41+
IL_0004: ldarga arg
42+
IL_0008: call int32 NonGenericConstrainedCallVirt::Bar(class C&)
43+
IL_000d: constrained. C
44+
IL_0013: callvirt instance void C::Baz(int32)
45+
IL_0018: ret
46+
} // end of method NonGenericConstrainedCallVirt::Foo
47+
48+
.method private hidebysig static
49+
int32 Bar (
50+
class C& o
51+
) cil managed
52+
{
53+
// Method begins at RVA 0x2080
54+
// Code size 14 (0xe)
55+
.maxstack 8
56+
57+
IL_0000: ldarg.0
58+
IL_0001: ldstr "B"
59+
IL_0006: newobj instance void C::.ctor(string)
60+
IL_000b: stind.ref
61+
IL_000c: ldc.i4.0
62+
IL_000d: ret
63+
} // end of method NonGenericConstrainedCallVirt::Bar
64+
65+
.method public hidebysig specialname rtspecialname
66+
instance void .ctor () cil managed
67+
{
68+
// Method begins at RVA 0x2090
69+
// Code size 7 (0x7)
70+
.maxstack 8
71+
72+
IL_0000: ldarg.0
73+
IL_0001: call instance void [mscorlib]System.Object::.ctor()
74+
IL_0006: ret
75+
} // end of method NonGenericConstrainedCallVirt::.ctor
76+
77+
} // end of class NonGenericConstrainedCallVirt
78+
79+
.class public auto ansi beforefieldinit C
80+
extends [mscorlib]System.Object
81+
{
82+
// Fields
83+
.field private initonly string '<Name>k__BackingField'
84+
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
85+
01 00 00 00
86+
)
87+
88+
// Methods
89+
.method public hidebysig specialname rtspecialname
90+
instance void .ctor (
91+
string n
92+
) cil managed
93+
{
94+
// Method begins at RVA 0x2098
95+
// Code size 14 (0xe)
96+
.maxstack 8
97+
98+
IL_0000: ldarg.0
99+
IL_0001: call instance void [mscorlib]System.Object::.ctor()
100+
IL_0006: ldarg.0
101+
IL_0007: ldarg.1
102+
IL_0008: stfld string C::'<Name>k__BackingField'
103+
IL_000d: ret
104+
} // end of method C::.ctor
105+
106+
.method public hidebysig
107+
instance void Baz (
108+
int32 arg
109+
) cil managed
110+
{
111+
// Method begins at RVA 0x20a8
112+
// Code size 12 (0xc)
113+
.maxstack 8
114+
115+
IL_0000: ldarg.0
116+
IL_0001: call instance string C::get_Name()
117+
IL_0006: call void [mscorlib]System.Console::WriteLine(string)
118+
IL_000b: ret
119+
} // end of method C::Baz
120+
121+
.method public hidebysig specialname
122+
instance string get_Name () cil managed
123+
{
124+
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
125+
01 00 00 00
126+
)
127+
// Method begins at RVA 0x20b8
128+
// Code size 7 (0x7)
129+
.maxstack 8
130+
131+
IL_0000: ldarg.0
132+
IL_0001: ldfld string C::'<Name>k__BackingField'
133+
IL_0006: ret
134+
} // end of method C::get_Name
135+
136+
// Properties
137+
.property instance string Name()
138+
{
139+
.get instance string C::get_Name()
140+
}
141+
142+
} // end of class C
143+

‎ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,5 +296,11 @@ public static string Issue2231b<T>()
296296
{
297297
return default(T).ToString();
298298
}
299+
300+
public static void ConstrainedCall<T>(T x, ref T y) where T : IDisposable
301+
{
302+
x.Dispose();
303+
y.Dispose();
304+
}
299305
}
300306
}

‎ICSharpCode.Decompiler/CSharp/CallBuilder.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,11 +356,18 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method,
356356
}
357357
else
358358
{
359+
var thisArg = callArguments.FirstOrDefault();
360+
if (thisArg is LdObjIfRef ldObjIfRef)
361+
{
362+
Debug.Assert(constrainedTo != null);
363+
thisArg = ldObjIfRef.Target;
364+
}
359365
target = expressionBuilder.TranslateTarget(
360-
callArguments.FirstOrDefault(),
366+
thisArg,
361367
nonVirtualInvocation: callOpCode == OpCode.Call || method.IsConstructor,
362368
memberStatic: method.IsStatic,
363-
memberDeclaringType: constrainedTo ?? method.DeclaringType);
369+
memberDeclaringType: method.DeclaringType,
370+
constrainedTo: constrainedTo);
364371
if (constrainedTo == null
365372
&& target.Expression is CastExpression cast
366373
&& target.ResolveResult is ConversionResolveResult conversion

‎ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2614,7 +2614,7 @@ protected internal override TranslatedExpression VisitBlockContainer(BlockContai
26142614
}
26152615

26162616
internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirtualInvocation,
2617-
bool memberStatic, IType memberDeclaringType)
2617+
bool memberStatic, IType memberDeclaringType, IType constrainedTo = null)
26182618
{
26192619
// If references are missing member.IsStatic might not be set correctly.
26202620
// Additionally check target for null, in order to avoid a crash.
@@ -2630,8 +2630,8 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26302630
}
26312631
else
26322632
{
2633-
IType targetTypeHint = memberDeclaringType;
2634-
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref)
2633+
IType targetTypeHint = constrainedTo ?? memberDeclaringType;
2634+
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType, constrainedTo) == StackType.Ref)
26352635
{
26362636
if (target.ResultType == StackType.Ref)
26372637
{
@@ -2643,13 +2643,13 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26432643
}
26442644
}
26452645
var translatedTarget = Translate(target, targetTypeHint);
2646-
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref)
2646+
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType, constrainedTo) == StackType.Ref)
26472647
{
26482648
// When accessing members on value types, ensure we use a reference of the correct type,
26492649
// and not a pointer or a reference to a different type (issue #1333)
2650-
if (!(translatedTarget.Type is ByReferenceType brt && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(brt.ElementType, memberDeclaringType)))
2650+
if (!(translatedTarget.Type is ByReferenceType brt && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(brt.ElementType, constrainedTo ?? memberDeclaringType)))
26512651
{
2652-
translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(memberDeclaringType), this);
2652+
translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(constrainedTo ?? memberDeclaringType), this);
26532653
}
26542654
}
26552655
if (translatedTarget.Expression is DirectionExpression)
@@ -2675,9 +2675,9 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26752675
}
26762676
else
26772677
{
2678-
return new TypeReferenceExpression(ConvertType(memberDeclaringType))
2678+
return new TypeReferenceExpression(ConvertType(constrainedTo ?? memberDeclaringType))
26792679
.WithoutILInstruction()
2680-
.WithRR(new TypeResolveResult(memberDeclaringType));
2680+
.WithRR(new TypeResolveResult(constrainedTo ?? memberDeclaringType));
26812681
}
26822682

26832683
bool ShouldUseBaseReference()
@@ -2686,7 +2686,7 @@ bool ShouldUseBaseReference()
26862686
return false;
26872687
if (!MatchLdThis(target))
26882688
return false;
2689-
if (memberDeclaringType.GetDefinition() == resolver.CurrentTypeDefinition)
2689+
if ((constrainedTo ?? memberDeclaringType).GetDefinition() == resolver.CurrentTypeDefinition)
26902690
return false;
26912691
return true;
26922692
}

‎ICSharpCode.Decompiler/IL/ILReader.cs‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,15 +1766,36 @@ ILInstruction[] PrepareArguments(bool firstArgumentIsStObjTarget)
17661766
{
17671767
int firstArgument = (opCode != OpCode.NewObj && !method.IsStatic) ? 1 : 0;
17681768
var arguments = new ILInstruction[firstArgument + method.Parameters.Count];
1769+
IType typeOfThis = constrainedPrefix ?? method.DeclaringType;
1770+
StackType expectedStackType = CallInstruction.ExpectedTypeForThisPointer(method.DeclaringType, constrainedPrefix);
1771+
bool requiresLdObjIfRef = firstArgument == 1
1772+
&& !firstArgumentIsStObjTarget
1773+
&& expectedStackType == StackType.Ref && typeOfThis.IsReferenceType != false;
17691774
for (int i = method.Parameters.Count - 1; i >= 0; i--)
17701775
{
1776+
if (requiresLdObjIfRef)
1777+
{
1778+
FlushExpressionStack();
1779+
}
1780+
17711781
arguments[firstArgument + i] = Pop(method.Parameters[i].Type.GetStackType());
17721782
}
17731783
if (firstArgument == 1)
17741784
{
1775-
arguments[0] = firstArgumentIsStObjTarget
1776-
? PopStObjTarget()
1777-
: Pop(CallInstruction.ExpectedTypeForThisPointer(constrainedPrefix ?? method.DeclaringType));
1785+
ILInstruction firstArgumentInstruction;
1786+
if (firstArgumentIsStObjTarget)
1787+
{
1788+
firstArgumentInstruction = PopStObjTarget();
1789+
}
1790+
else
1791+
{
1792+
firstArgumentInstruction = Pop(expectedStackType);
1793+
if (requiresLdObjIfRef)
1794+
{
1795+
firstArgumentInstruction = new LdObjIfRef(firstArgumentInstruction, typeOfThis);
1796+
}
1797+
}
1798+
arguments[0] = firstArgumentInstruction;
17781799
}
17791800
// arguments is in reverse order of the Pop calls, thus
17801801
// arguments is now in the correct evaluation order.

0 commit comments

Comments
 (0)