Skip to content

Commit 39e9b10

Browse files
MarcoRossignoliMarcoRossignoli
authored
Skip lambda cached field (#753)
Skip lambda cached field
1 parent 54e576f commit 39e9b10

File tree

4 files changed

+233
-127
lines changed

4 files changed

+233
-127
lines changed

‎src/coverlet.core/Symbols/CecilSymbolHelper.cs‎

Lines changed: 156 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ private static bool IsCompilerGenerated(MethodDefinition methodDefinition)
3636
return false;
3737
}
3838

39+
private static bool IsCompilerGenerated(FieldDefinition fieldDefinition)
40+
{
41+
return fieldDefinition.DeclaringType.CustomAttributes.Any(ca => ca.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName);
42+
}
43+
3944
private static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition)
4045
{
4146
if (methodDefinition.FullName.EndsWith("::MoveNext()") && IsCompilerGenerated(methodDefinition))
@@ -72,14 +77,15 @@ private static bool IsMoveNextInsideEnumerator(MethodDefinition methodDefinition
7277
return false;
7378
}
7479

75-
private static bool IsRecognizedMoveNextInsideAsyncStateMachineProlog(MethodDefinition methodDefinition)
80+
private static bool IsMoveNextInsideAsyncStateMachineProlog(MethodDefinition methodDefinition)
7681
{
7782
/*
7883
int num = <>1__state;
7984
IL_0000: ldarg.0
8085
IL_0001: ldfld ...::'<>1__state'
8186
IL_0006: stloc.0
8287
*/
88+
8389
return (methodDefinition.Body.Instructions[0].OpCode == OpCodes.Ldarg_0 ||
8490
methodDefinition.Body.Instructions[0].OpCode == OpCodes.Ldarg) &&
8591

@@ -92,17 +98,148 @@ private static bool IsRecognizedMoveNextInsideAsyncStateMachineProlog(MethodDefi
9298
methodDefinition.Body.Instructions[2].OpCode == OpCodes.Stloc_0;
9399
}
94100

101+
private static bool SkipMoveNextPrologueBranches(Instruction instruction)
102+
{
103+
/*
104+
If method is a generated MoveNext we'll skip first branches (could be a switch or a series of branches)
105+
that check state machine value to jump to correct state (for instance after a true async call)
106+
Check if it's a Cond_Branch on state machine current value int num = <>1__state;
107+
We are on branch OpCode so we need to go back by max 2 operation to reach ldloc.0 the load of "num"
108+
Max 2 because we handle following patterns
109+
110+
Swich
111+
112+
// switch (num)
113+
IL_0007: ldloc.0 2
114+
// (no C# code)
115+
IL_0008: switch (IL_0037, IL_003c, ... 1
116+
...
117+
118+
Single branch
119+
120+
// if (num != 0)
121+
IL_0007: ldloc.0 2
122+
// (no C# code)
123+
IL_0008: brfalse.s IL_000c 1
124+
IL_000a: br.s IL_000e
125+
IL_000c: br.s IL_0049
126+
IL_000e: nop
127+
...
128+
129+
More tha one branch
130+
131+
// if (num != 0)
132+
IL_0007: ldloc.0
133+
// (no C# code)
134+
IL_0008: brfalse.s IL_0012
135+
IL_000a: br.s IL_000c
136+
// if (num == 1)
137+
IL_000c: ldloc.0 3
138+
IL_000d: ldc.i4.1 2
139+
IL_000e: beq.s IL_0014 1
140+
// (no C# code)
141+
IL_0010: br.s IL_0019
142+
IL_0012: br.s IL_0060
143+
IL_0014: br IL_00e5
144+
IL_0019: nop
145+
...
146+
147+
so we know that current branch are checking that field and we're not interested in.
148+
*/
149+
150+
Instruction current = instruction.Previous;
151+
for (int instructionBefore = 2; instructionBefore > 0 && current.Previous != null; current = current.Previous, instructionBefore--)
152+
{
153+
if (
154+
(current.OpCode == OpCodes.Ldloc && current.Operand is VariableDefinition vo && vo.Index == 0) ||
155+
current.OpCode == OpCodes.Ldloc_0
156+
)
157+
{
158+
return true;
159+
}
160+
}
161+
return false;
162+
}
163+
164+
private static bool SkipIsCompleteAwaiters(Instruction instruction)
165+
{
166+
// Skip get_IsCompleted to avoid unuseful branch due to async/await state machine
167+
if (
168+
instruction.Previous.Operand is MethodReference operand &&
169+
operand.Name == "get_IsCompleted" &&
170+
(
171+
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.TaskAwaiter") ||
172+
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.ConfiguredTaskAwaitable") ||
173+
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable")
174+
)
175+
&&
176+
(
177+
operand.DeclaringType.Scope.Name == "System.Runtime" ||
178+
operand.DeclaringType.Scope.Name == "netstandard" ||
179+
operand.DeclaringType.Scope.Name == "System.Threading.Tasks.Extensions"
180+
)
181+
)
182+
{
183+
return true;
184+
}
185+
return false;
186+
}
187+
188+
private static bool SkipLambdaCachedField(Instruction instruction)
189+
{
190+
/*
191+
Lambda cached field pattern
192+
193+
IL_0074: ldloca.s 1
194+
IL_0076: call instance void [System.Runtime]System.Runtime.CompilerServices.TaskAwaiter::GetResult()
195+
IL_007b: nop
196+
IL_007c: ldarg.0
197+
IL_007d: ldarg.0
198+
IL_007e: ldfld class [System.Runtime]System.Collections.Generic.IEnumerable`1<object> Coverlet.Core.Samples.Tests.Issue_730/'<DoSomethingAsyncWithLinq>d__1'::objects
199+
IL_0083: ldsfld class [System.Runtime]System.Func`2<object, object> Coverlet.Core.Samples.Tests.Issue_730/'<>c'::'<>9__1_0'
200+
IL_0088: dup
201+
IL_0089: brtrue.s IL_00a2 -> CHECK IF CACHED FIELD IS NULL OR JUMP TO DELEGATE USAGE
202+
203+
(INIT STATIC FIELD)
204+
IL_008b: pop
205+
IL_008c: ldsfld class Coverlet.Core.Samples.Tests.Issue_730/'<>c' Coverlet.Core.Samples.Tests.Issue_730/'<>c'::'<>9'
206+
IL_0091: ldftn instance object Coverlet.Core.Samples.Tests.Issue_730/'<>c'::'<DoSomethingAsyncWithLinq>b__1_0'(object)
207+
IL_0097: newobj instance void class [System.Runtime]System.Func`2<object, object>::.ctor(object, native int)
208+
IL_009c: dup
209+
IL_009d: stsfld class [System.Runtime]System.Func`2<object, object> Coverlet.Core.Samples.Tests.Issue_730/'<>c'::'<>9__1_0'
210+
211+
(USE DELEGATE FIELD)
212+
IL_00a2: call class [System.Runtime]System.Collections.Generic.IEnumerable`1<!!1> [System.Linq]System.Linq.Enumerable::Select<object, object>(class [System.Runtime]System.Collections.Generic.IEnumerable`1<!!0>, class [System.Runtime]System.Func`2<!!0, !!1>)
213+
*/
214+
215+
Instruction current = instruction.Previous;
216+
for (int instructionBefore = 2; instructionBefore > 0 && current.Previous != null; current = current.Previous, instructionBefore--)
217+
{
218+
if (current.OpCode == OpCodes.Ldsfld && current.Operand is FieldDefinition fd &&
219+
// LambdaCacheField https://github.com/dotnet/roslyn/blob/e704ca635bd6de70a0250e34c4567c7a28fa9f6d/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNameKind.cs#L31
220+
// https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNames.cs#L145
221+
fd.Name.StartsWith("<>9_") &&
222+
IsCompilerGenerated(fd))
223+
{
224+
return true;
225+
}
226+
}
227+
return false;
228+
}
229+
95230
public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
96231
{
97232
var list = new List<BranchPoint>();
98-
if (methodDefinition == null)
233+
if (methodDefinition is null)
234+
{
99235
return list;
236+
}
100237

101-
UInt32 ordinal = 0;
238+
uint ordinal = 0;
102239
var instructions = methodDefinition.Body.Instructions;
103240

104241
bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition);
105-
bool isRecognizedMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsRecognizedMoveNextInsideAsyncStateMachineProlog(methodDefinition);
242+
bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition);
106243
bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition);
107244

108245
foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch))
@@ -115,98 +252,28 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
115252
continue;
116253
}
117254

118-
/*
119-
If method is a generated MoveNext we'll skip first branches (could be a switch or a series of branches)
120-
that check state machine value to jump to correct state (for instance after a true async call)
121-
Check if it's a Cond_Branch on state machine current value int num = <>1__state;
122-
We are on branch OpCode so we need to go back by max 2 operation to reach ldloc.0 the load of "num"
123-
Max 2 because we handle following patterns
124-
125-
Swich
126-
127-
// switch (num)
128-
IL_0007: ldloc.0 2
129-
// (no C# code)
130-
IL_0008: switch (IL_0037, IL_003c, ... 1
131-
...
132-
133-
Single branch
134-
135-
// if (num != 0)
136-
IL_0007: ldloc.0 2
137-
// (no C# code)
138-
IL_0008: brfalse.s IL_000c 1
139-
IL_000a: br.s IL_000e
140-
IL_000c: br.s IL_0049
141-
IL_000e: nop
142-
...
143-
144-
More tha one branch
145-
146-
// if (num != 0)
147-
IL_0007: ldloc.0
148-
// (no C# code)
149-
IL_0008: brfalse.s IL_0012
150-
IL_000a: br.s IL_000c
151-
// if (num == 1)
152-
IL_000c: ldloc.0 3
153-
IL_000d: ldc.i4.1 2
154-
IL_000e: beq.s IL_0014 1
155-
// (no C# code)
156-
IL_0010: br.s IL_0019
157-
IL_0012: br.s IL_0060
158-
IL_0014: br IL_00e5
159-
IL_0019: nop
160-
...
161-
162-
so we know that current branch are checking that field and we're not interested in.
163-
*/
164-
if (isRecognizedMoveNextInsideAsyncStateMachineProlog)
255+
if (isMoveNextInsideAsyncStateMachineProlog)
165256
{
166-
bool skipInstruction = false;
167-
Instruction current = instruction.Previous;
168-
for (int instructionBefore = 2; instructionBefore > 0 && current.Previous != null; current = current.Previous, instructionBefore--)
169-
{
170-
if (
171-
(current.OpCode == OpCodes.Ldloc && current.Operand is VariableDefinition vo && vo.Index == 0) ||
172-
current.OpCode == OpCodes.Ldloc_0
173-
)
174-
{
175-
skipInstruction = true;
176-
break;
177-
}
178-
}
179-
if (skipInstruction)
257+
if (SkipMoveNextPrologueBranches(instruction) || SkipIsCompleteAwaiters(instruction))
180258
{
181259
continue;
182260
}
183261
}
184262

185-
// Skip get_IsCompleted to avoid unuseful branch due to async/await state machine
186-
if (
187-
isRecognizedMoveNextInsideAsyncStateMachineProlog && instruction.Previous.Operand is MethodReference operand &&
188-
operand.Name == "get_IsCompleted" &&
189-
(
190-
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.TaskAwaiter") ||
191-
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.ConfiguredTaskAwaitable") ||
192-
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable")
193-
)
194-
&&
195-
(
196-
operand.DeclaringType.Scope.Name == "System.Runtime" ||
197-
operand.DeclaringType.Scope.Name == "netstandard" ||
198-
operand.DeclaringType.Scope.Name == "System.Threading.Tasks.Extensions"
199-
)
200-
)
263+
if (SkipLambdaCachedField(instruction))
201264
{
202265
continue;
203266
}
204267

205-
if (BranchIsInGeneratedExceptionFilter(instruction, methodDefinition))
268+
if (SkipBranchGeneratedExceptionFilter(instruction, methodDefinition))
269+
{
206270
continue;
271+
}
207272

208-
if (BranchIsInGeneratedFinallyBlock(instruction, methodDefinition))
273+
if (SkipBranchGeneratedFinallyBlock(instruction, methodDefinition))
274+
{
209275
continue;
276+
}
210277

211278
var pathCounter = 0;
212279

@@ -217,10 +284,14 @@ so we know that current branch are checking that field and we're not interested
217284
var document = closestSeqPt.Maybe(x => x.Document.Url);
218285

219286
if (instruction.Next == null)
287+
{
220288
return list;
289+
}
221290

222291
if (!BuildPointsForConditionalBranch(list, instruction, branchingInstructionLine, document, branchOffset, pathCounter, instructions, ref ordinal, methodDefinition))
292+
{
223293
return list;
294+
}
224295
}
225296
catch (Exception)
226297
{
@@ -360,7 +431,7 @@ private static uint BuildPointsForSwitchCases(List<BranchPoint> list, BranchPoin
360431
return ordinal;
361432
}
362433

363-
private static bool BranchIsInGeneratedExceptionFilter(Instruction branchInstruction, MethodDefinition methodDefinition)
434+
private static bool SkipBranchGeneratedExceptionFilter(Instruction branchInstruction, MethodDefinition methodDefinition)
364435
{
365436
if (!methodDefinition.Body.HasExceptionHandlers)
366437
return false;
@@ -389,7 +460,7 @@ private static bool BranchIsInGeneratedExceptionFilter(Instruction branchInstruc
389460
return false;
390461
}
391462

392-
private static bool BranchIsInGeneratedFinallyBlock(Instruction branchInstruction, MethodDefinition methodDefinition)
463+
private static bool SkipBranchGeneratedFinallyBlock(Instruction branchInstruction, MethodDefinition methodDefinition)
393464
{
394465
if (!methodDefinition.Body.HasExceptionHandlers)
395466
return false;

0 commit comments

Comments
 (0)