Browse Source

Fix #3110: Add support for MCS 2.6.4 pinned region with array variable

* Added additional code to remove the conv instruction present in the initialization part of the pinned region.
* Extended the code responsible for removing the unpin stloc to correctly match the inverted condition found in MCS 2.6.4 compiled code.
* Enabled already present correctness test to run for MCS 2.6.4.

This is a more generalized version of the fix on PR #3110 proposed by @ElektroKill.
pull/3195/head v9.0-preview1
Daniel Grunwald 1 year ago
parent
commit
38e7ab4373
  1. 4
      ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs
  2. 41
      ICSharpCode.Decompiler/IL/ControlFlow/DetectPinnedRegions.cs
  3. 2
      ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs
  4. 3
      ICSharpCode.Decompiler/IL/Transforms/Stepper.cs

4
ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@ -336,10 +336,6 @@ namespace ICSharpCode.Decompiler.Tests @@ -336,10 +336,6 @@ namespace ICSharpCode.Decompiler.Tests
[Test]
public async Task UnsafeCode([ValueSource(nameof(defaultOptions))] CompilerOptions options)
{
if (options.HasFlag(CompilerOptions.UseMcs2_6_4))
{
Assert.Ignore("Decompiler bug with mono!");
}
await RunCS(options: options);
}

41
ICSharpCode.Decompiler/IL/ControlFlow/DetectPinnedRegions.cs

@ -19,6 +19,7 @@ @@ -19,6 +19,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Runtime.InteropServices.ComTypes;
using ICSharpCode.Decompiler.IL.Transforms;
using ICSharpCode.Decompiler.TypeSystem;
@ -612,18 +613,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -612,18 +613,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
innerBlock = (Block)innerBlock.Clone();
clonedBlocks[i] = innerBlock;
}
Branch br = innerBlock.Instructions.LastOrDefault() as Branch;
if (br != null && br.TargetBlock.IncomingEdgeCount == 1
&& br.TargetContainer == sourceContainer && reachedEdgesPerBlock[br.TargetBlock.ChildIndex] == 0)
if (innerBlock.MatchIfAtEndOfBlock(out _, out var trueInst, out var falseInst))
{
// branch that leaves body.
// The target block should have an instruction that resets the pin; delete that instruction:
StLoc unpin = br.TargetBlock.Instructions.First() as StLoc;
if (unpin != null && unpin.Variable == stLoc.Variable && IsNullOrZero(unpin.Value))
{
br.TargetBlock.Instructions.RemoveAt(0);
}
HandleBranchLeavingPinnedRegion(trueInst, reachedEdgesPerBlock, sourceContainer, stLoc.Variable);
HandleBranchLeavingPinnedRegion(falseInst, reachedEdgesPerBlock, sourceContainer, stLoc.Variable);
}
else
{
HandleBranchLeavingPinnedRegion(innerBlock.Instructions.LastOrDefault(), reachedEdgesPerBlock, sourceContainer, stLoc.Variable);
}
// move block into body
if (sourceContainer.Blocks[i] == entryBlock)
{
@ -698,6 +697,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -698,6 +697,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
return true;
}
static void HandleBranchLeavingPinnedRegion(ILInstruction potentialBranch, int[] reachedEdgesPerBlock, BlockContainer sourceContainer, ILVariable pinnedRegionVar)
{
if (potentialBranch is Branch branch && branch.TargetBlock.IncomingEdgeCount == 1
&& branch.TargetContainer == sourceContainer && reachedEdgesPerBlock[branch.TargetBlock.ChildIndex] == 0)
{
// branch that leaves body.
// The target block should have an instruction that resets the pin; delete that instruction:
StLoc unpin = branch.TargetBlock.Instructions.First() as StLoc;
if (unpin != null && unpin.Variable == pinnedRegionVar && IsNullOrZero(unpin.Value))
{
branch.TargetBlock.Instructions.RemoveAt(0);
}
}
}
static bool IsNullOrZero(ILInstruction inst)
{
while (inst is Conv conv)
@ -750,6 +764,13 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -750,6 +764,13 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
// fixing a string
HandleStringToPointer(pinnedRegion);
}
else if (pinnedRegion.Init is Conv { Kind: ConversionKind.StopGCTracking, Argument: var convArg })
{
// If pinnedRegion.Variable was already a pointer type, the input IL has a StopGCTracking conversion.
// We can simply remove this conversion, as it is not needed.
context.Step("Remove StopGCTracking conversion", pinnedRegion);
pinnedRegion.Init = convArg;
}
// Detect nested pinned regions:
BlockContainer body = (BlockContainer)pinnedRegion.Body;
foreach (var block in body.Blocks)

2
ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs

@ -88,12 +88,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -88,12 +88,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// Unlike <c>context.Stepper.Step()</c>, calls to this method are only compiled in debug builds.
/// </summary>
[Conditional("STEP")]
[DebuggerStepThrough]
internal void Step(string description, ILInstruction? near)
{
Stepper.Step(description, near);
}
[Conditional("STEP")]
[DebuggerStepThrough]
internal void StepStartGroup(string description, ILInstruction? near = null)
{
Stepper.StartGroup(description, near);

3
ICSharpCode.Decompiler/IL/Transforms/Stepper.cs

@ -95,11 +95,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -95,11 +95,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms
///
/// May throw <see cref="StepLimitReachedException"/> in debug mode.
/// </summary>
[DebuggerStepThrough]
public void Step(string description, ILInstruction? near = null)
{
StepInternal(description, near);
}
[DebuggerStepThrough]
private Node StepInternal(string description, ILInstruction? near)
{
if (step == StepLimit)
@ -123,6 +125,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -123,6 +125,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return stepNode;
}
[DebuggerStepThrough]
public void StartGroup(string description, ILInstruction? near = null)
{
groups.Push(StepInternal(description, near));

Loading…
Cancel
Save