Browse Source

Fix #1386: MoveArrayToPointerToPinnedRegionInit: Only consider the pinned variable uses within the PinnedRegion

This also makes CleanUpTryFinallyAroundPinnedRegion() redundant as it is no longer necessary to trigger the array-to-pointer transform; so the normal elimination of pinned variable resets is sufficient.
pull/1423/head
Daniel Grunwald 6 years ago
parent
commit
f6aae1f97d
  1. 9
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs
  2. 22
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs
  3. 110
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.il
  4. 98
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.il
  5. 85
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.roslyn.il
  6. 107
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.roslyn.il
  7. 14
      ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoArrayInitializers.Expected.cs
  8. 56
      ICSharpCode.Decompiler/IL/ControlFlow/DetectPinnedRegions.cs
  9. 5
      ICSharpCode.Decompiler/IL/ILTypeExtensions.cs

9
ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs

@ -1123,21 +1123,12 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.InitializerTests @@ -1123,21 +1123,12 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.InitializerTests
public static byte[] ReverseInitializer(int i)
{
#if ROSLYN && OPT
byte[] obj = new byte[4];
obj[3] = (byte)i;
obj[2] = (byte)(i >> 8);
obj[1] = (byte)(i >> 16);
obj[0] = (byte)(i >> 24);
return obj;
#else
byte[] array = new byte[4];
array[3] = (byte)i;
array[2] = (byte)(i >> 8);
array[1] = (byte)(i >> 16);
array[0] = (byte)(i >> 24);
return array;
#endif
}
public static void Issue953_MissingNullableSpecifierForArrayInitializer()

22
ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs

@ -417,5 +417,27 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -417,5 +417,27 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine("Finally");
}
}
private unsafe static object CreateBuffer(int length, byte* ptr)
{
throw new NotImplementedException();
}
private unsafe static object Issue1386(int arraySize, bool createFirstBuffer)
{
if (createFirstBuffer) {
byte[] array = new byte[arraySize];
Console.WriteLine("first fixed");
fixed (byte* ptr = array) {
return CreateBuffer(array.Length, ptr);
}
}
byte[] array2 = new byte[arraySize];
Console.WriteLine("second fixed");
fixed (byte* ptr2 = array2) {
return CreateBuffer(array2.Length, ptr2);
}
}
}
}

110
ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.il

@ -1593,6 +1593,116 @@ @@ -1593,6 +1593,116 @@
IL_005a: ret
} // end of method UnsafeCode::NestedFixedBlocks
.method private hidebysig static object
CreateBuffer(int32 length,
uint8* ptr) cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: nop
IL_0001: newobj instance void [mscorlib]System.NotImplementedException::.ctor()
IL_0006: throw
} // end of method UnsafeCode::CreateBuffer
.method private hidebysig static object
Issue1386(int32 arraySize,
bool createFirstBuffer) cil managed
{
// Code size 134 (0x86)
.maxstack 2
.locals init (uint8[] V_0,
uint8& pinned V_1,
uint8[] V_2,
uint8& pinned V_3,
object V_4,
bool V_5,
uint8[] V_6)
IL_0000: nop
IL_0001: ldarg.1
IL_0002: ldc.i4.0
IL_0003: ceq
IL_0005: stloc.s V_5
IL_0007: ldloc.s V_5
IL_0009: brtrue.s IL_0047
IL_000b: nop
IL_000c: ldarg.0
IL_000d: newarr [mscorlib]System.Byte
IL_0012: stloc.0
IL_0013: ldstr "first fixed"
IL_0018: call void [mscorlib]System.Console::WriteLine(string)
IL_001d: nop
IL_001e: ldloc.0
IL_001f: dup
IL_0020: stloc.s V_6
IL_0022: brfalse.s IL_002a
IL_0024: ldloc.s V_6
IL_0026: ldlen
IL_0027: conv.i4
IL_0028: brtrue.s IL_002f
IL_002a: ldc.i4.0
IL_002b: conv.u
IL_002c: stloc.1
IL_002d: br.s IL_0038
IL_002f: ldloc.s V_6
IL_0031: ldc.i4.0
IL_0032: ldelema [mscorlib]System.Byte
IL_0037: stloc.1
IL_0038: nop
IL_0039: ldloc.0
IL_003a: ldlen
IL_003b: conv.i4
IL_003c: ldloc.1
IL_003d: conv.i
IL_003e: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_0043: stloc.s V_4
IL_0045: leave.s IL_0082
IL_0047: ldarg.0
IL_0048: newarr [mscorlib]System.Byte
IL_004d: stloc.2
IL_004e: ldstr "second fixed"
IL_0053: call void [mscorlib]System.Console::WriteLine(string)
IL_0058: nop
IL_0059: ldloc.2
IL_005a: dup
IL_005b: stloc.s V_6
IL_005d: brfalse.s IL_0065
IL_005f: ldloc.s V_6
IL_0061: ldlen
IL_0062: conv.i4
IL_0063: brtrue.s IL_006a
IL_0065: ldc.i4.0
IL_0066: conv.u
IL_0067: stloc.3
IL_0068: br.s IL_0073
IL_006a: ldloc.s V_6
IL_006c: ldc.i4.0
IL_006d: ldelema [mscorlib]System.Byte
IL_0072: stloc.3
IL_0073: nop
IL_0074: ldloc.2
IL_0075: ldlen
IL_0076: conv.i4
IL_0077: ldloc.3
IL_0078: conv.i
IL_0079: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_007e: stloc.s V_4
IL_0080: leave.s IL_0082
IL_0082: nop
IL_0083: ldloc.s V_4
IL_0085: ret
} // end of method UnsafeCode::Issue1386
.property instance int32* NullPointer()
{
.get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

98
ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.il

@ -1289,6 +1289,104 @@ @@ -1289,6 +1289,104 @@
IL_0055: ret
} // end of method UnsafeCode::NestedFixedBlocks
.method private hidebysig static object
CreateBuffer(int32 length,
uint8* ptr) cil managed
{
// Code size 6 (0x6)
.maxstack 8
IL_0000: newobj instance void [mscorlib]System.NotImplementedException::.ctor()
IL_0005: throw
} // end of method UnsafeCode::CreateBuffer
.method private hidebysig static object
Issue1386(int32 arraySize,
bool createFirstBuffer) cil managed
{
// Code size 120 (0x78)
.maxstack 2
.locals init (uint8[] V_0,
uint8& pinned V_1,
uint8[] V_2,
uint8& pinned V_3,
object V_4,
uint8[] V_5,
uint8[] V_6)
IL_0000: ldarg.1
IL_0001: brfalse.s IL_003c
IL_0003: ldarg.0
IL_0004: newarr [mscorlib]System.Byte
IL_0009: stloc.0
IL_000a: ldstr "first fixed"
IL_000f: call void [mscorlib]System.Console::WriteLine(string)
IL_0014: ldloc.0
IL_0015: dup
IL_0016: stloc.s V_5
IL_0018: brfalse.s IL_0020
IL_001a: ldloc.s V_5
IL_001c: ldlen
IL_001d: conv.i4
IL_001e: brtrue.s IL_0025
IL_0020: ldc.i4.0
IL_0021: conv.u
IL_0022: stloc.1
IL_0023: br.s IL_002e
IL_0025: ldloc.s V_5
IL_0027: ldc.i4.0
IL_0028: ldelema [mscorlib]System.Byte
IL_002d: stloc.1
IL_002e: ldloc.0
IL_002f: ldlen
IL_0030: conv.i4
IL_0031: ldloc.1
IL_0032: conv.i
IL_0033: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_0038: stloc.s V_4
IL_003a: leave.s IL_0075
IL_003c: ldarg.0
IL_003d: newarr [mscorlib]System.Byte
IL_0042: stloc.2
IL_0043: ldstr "second fixed"
IL_0048: call void [mscorlib]System.Console::WriteLine(string)
IL_004d: ldloc.2
IL_004e: dup
IL_004f: stloc.s V_6
IL_0051: brfalse.s IL_0059
IL_0053: ldloc.s V_6
IL_0055: ldlen
IL_0056: conv.i4
IL_0057: brtrue.s IL_005e
IL_0059: ldc.i4.0
IL_005a: conv.u
IL_005b: stloc.3
IL_005c: br.s IL_0067
IL_005e: ldloc.s V_6
IL_0060: ldc.i4.0
IL_0061: ldelema [mscorlib]System.Byte
IL_0066: stloc.3
IL_0067: ldloc.2
IL_0068: ldlen
IL_0069: conv.i4
IL_006a: ldloc.3
IL_006b: conv.i
IL_006c: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_0071: stloc.s V_4
IL_0073: leave.s IL_0075
IL_0075: ldloc.s V_4
IL_0077: ret
} // end of method UnsafeCode::Issue1386
.property instance int32* NullPointer()
{
.get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

85
ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.opt.roslyn.il

@ -1298,6 +1298,91 @@ @@ -1298,6 +1298,91 @@
IL_0053: ret
} // end of method UnsafeCode::NestedFixedBlocks
.method private hidebysig static object
CreateBuffer(int32 length,
uint8* ptr) cil managed
{
// Code size 6 (0x6)
.maxstack 8
IL_0000: newobj instance void [mscorlib]System.NotImplementedException::.ctor()
IL_0005: throw
} // end of method UnsafeCode::CreateBuffer
.method private hidebysig static object
Issue1386(int32 arraySize,
bool createFirstBuffer) cil managed
{
// Code size 101 (0x65)
.maxstack 3
.locals init (uint8* V_0,
uint8[] pinned V_1,
uint8* V_2)
IL_0000: ldarg.1
IL_0001: brfalse.s IL_0034
IL_0003: ldarg.0
IL_0004: newarr [mscorlib]System.Byte
IL_0009: ldstr "first fixed"
IL_000e: call void [mscorlib]System.Console::WriteLine(string)
IL_0013: dup
IL_0014: dup
IL_0015: stloc.1
IL_0016: brfalse.s IL_001d
IL_0018: ldloc.1
IL_0019: ldlen
IL_001a: conv.i4
IL_001b: brtrue.s IL_0022
IL_001d: ldc.i4.0
IL_001e: conv.u
IL_001f: stloc.0
IL_0020: br.s IL_002b
IL_0022: ldloc.1
IL_0023: ldc.i4.0
IL_0024: ldelema [mscorlib]System.Byte
IL_0029: conv.u
IL_002a: stloc.0
IL_002b: ldlen
IL_002c: conv.i4
IL_002d: ldloc.0
IL_002e: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_0033: ret
IL_0034: ldarg.0
IL_0035: newarr [mscorlib]System.Byte
IL_003a: ldstr "second fixed"
IL_003f: call void [mscorlib]System.Console::WriteLine(string)
IL_0044: dup
IL_0045: dup
IL_0046: stloc.1
IL_0047: brfalse.s IL_004e
IL_0049: ldloc.1
IL_004a: ldlen
IL_004b: conv.i4
IL_004c: brtrue.s IL_0053
IL_004e: ldc.i4.0
IL_004f: conv.u
IL_0050: stloc.2
IL_0051: br.s IL_005c
IL_0053: ldloc.1
IL_0054: ldc.i4.0
IL_0055: ldelema [mscorlib]System.Byte
IL_005a: conv.u
IL_005b: stloc.2
IL_005c: ldlen
IL_005d: conv.i4
IL_005e: ldloc.2
IL_005f: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_0064: ret
} // end of method UnsafeCode::Issue1386
.property instance int32* NullPointer()
{
.get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

107
ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.roslyn.il

@ -1595,6 +1595,113 @@ @@ -1595,6 +1595,113 @@
IL_0059: ret
} // end of method UnsafeCode::NestedFixedBlocks
.method private hidebysig static object
CreateBuffer(int32 length,
uint8* ptr) cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: nop
IL_0001: newobj instance void [mscorlib]System.NotImplementedException::.ctor()
IL_0006: throw
} // end of method UnsafeCode::CreateBuffer
.method private hidebysig static object
Issue1386(int32 arraySize,
bool createFirstBuffer) cil managed
{
// Code size 131 (0x83)
.maxstack 2
.locals init (uint8[] V_0,
bool V_1,
uint8[] V_2,
uint8* V_3,
uint8[] pinned V_4,
object V_5,
uint8* V_6)
IL_0000: nop
IL_0001: ldarg.1
IL_0002: stloc.1
IL_0003: ldloc.1
IL_0004: brfalse.s IL_0042
IL_0006: nop
IL_0007: ldarg.0
IL_0008: newarr [mscorlib]System.Byte
IL_000d: stloc.2
IL_000e: ldstr "first fixed"
IL_0013: call void [mscorlib]System.Console::WriteLine(string)
IL_0018: nop
IL_0019: ldloc.2
IL_001a: dup
IL_001b: stloc.s V_4
IL_001d: brfalse.s IL_0025
IL_001f: ldloc.s V_4
IL_0021: ldlen
IL_0022: conv.i4
IL_0023: brtrue.s IL_002a
IL_0025: ldc.i4.0
IL_0026: conv.u
IL_0027: stloc.3
IL_0028: br.s IL_0034
IL_002a: ldloc.s V_4
IL_002c: ldc.i4.0
IL_002d: ldelema [mscorlib]System.Byte
IL_0032: conv.u
IL_0033: stloc.3
IL_0034: nop
IL_0035: ldloc.2
IL_0036: ldlen
IL_0037: conv.i4
IL_0038: ldloc.3
IL_0039: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_003e: stloc.s V_5
IL_0040: br.s IL_0080
IL_0042: ldarg.0
IL_0043: newarr [mscorlib]System.Byte
IL_0048: stloc.0
IL_0049: ldstr "second fixed"
IL_004e: call void [mscorlib]System.Console::WriteLine(string)
IL_0053: nop
IL_0054: ldloc.0
IL_0055: dup
IL_0056: stloc.s V_4
IL_0058: brfalse.s IL_0060
IL_005a: ldloc.s V_4
IL_005c: ldlen
IL_005d: conv.i4
IL_005e: brtrue.s IL_0066
IL_0060: ldc.i4.0
IL_0061: conv.u
IL_0062: stloc.s V_6
IL_0064: br.s IL_0071
IL_0066: ldloc.s V_4
IL_0068: ldc.i4.0
IL_0069: ldelema [mscorlib]System.Byte
IL_006e: conv.u
IL_006f: stloc.s V_6
IL_0071: nop
IL_0072: ldloc.0
IL_0073: ldlen
IL_0074: conv.i4
IL_0075: ldloc.s V_6
IL_0077: call object ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::CreateBuffer(int32,
uint8*)
IL_007c: stloc.s V_5
IL_007e: br.s IL_0080
IL_0080: ldloc.s V_5
IL_0082: ret
} // end of method UnsafeCode::Issue1386
.property instance int32* NullPointer()
{
.get instance int32* ICSharpCode.Decompiler.Tests.TestCases.Pretty.UnsafeCode::get_NullPointer()

14
ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoArrayInitializers.Expected.cs

@ -7,17 +7,17 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly @@ -7,17 +7,17 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
{
public int[] LiteralArray()
{
int[] obj = new int[3];
RuntimeHelpers.InitializeArray(obj, (RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/);
return obj;
int[] array = new int[3];
RuntimeHelpers.InitializeArray(array, (RuntimeFieldHandle)/*OpCode not supported: LdMemberToken*/);
return array;
}
public int[] VariableArray(int a, int b)
{
int[] obj = new int[2];
obj[0] = a;
obj[1] = b;
return obj;
int[] array = new int[2];
array[0] = a;
array[1] = b;
return array;
}
}
}

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

@ -359,7 +359,6 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -359,7 +359,6 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
var pinnedRegion = new PinnedRegion(stLoc.Variable, stLoc.Value, body) { ILRange = stLoc.ILRange };
stLoc.ReplaceWith(pinnedRegion);
block.Instructions.RemoveAt(block.Instructions.Count - 1); // remove branch into body
CleanUpTryFinallyAroundPinnedRegion(pinnedRegion);
ProcessPinnedRegion(pinnedRegion);
return true;
}
@ -423,9 +422,17 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -423,9 +422,17 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
// Roslyn started marking the array variable as pinned,
// and then uses array.to.pointer immediately within the region.
Debug.Assert(pinnedRegion.Variable.Type.Kind == TypeKind.Array);
if (pinnedRegion.Variable.StoreInstructions.Count != 1 || pinnedRegion.Variable.AddressCount != 0 || pinnedRegion.Variable.LoadCount != 1)
return;
var ldloc = pinnedRegion.Variable.LoadInstructions.Single();
// Find the single load of the variable within the pinnedRegion:
LdLoc ldloc = null;
foreach (var inst in pinnedRegion.Descendants.OfType<IInstructionWithVariableOperand>()) {
if (inst.Variable == pinnedRegion.Variable && inst != pinnedRegion) {
if (ldloc != null)
return; // more than 1 variable access
ldloc = inst as LdLoc;
if (ldloc == null)
return; // variable access that is not LdLoc
}
}
if (!(ldloc.Parent is ArrayToPointer arrayToPointer))
return;
if (!(arrayToPointer.Parent is Conv conv && conv.Kind == ConversionKind.StopGCTracking))
@ -627,46 +634,5 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow @@ -627,46 +634,5 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
body.EntryPoint.Instructions.RemoveAt(0);
}
#endregion
#region CleanUpTryFinallyAroundPinnedRegion
private void CleanUpTryFinallyAroundPinnedRegion(PinnedRegion pinnedRegion)
{
if (!(pinnedRegion.Parent is Block tryBlock))
return;
if (!(tryBlock.Parent is BlockContainer tryContainer))
return;
if (!(tryContainer.Parent is TryFinally tryFinally))
return;
// .try BlockContainer {
// Block IL_001b (incoming: 1) {
// stloc S_11(call GetArray())
// PinnedRegion V_3(ldloc S_11, BlockContainer {
// Block IL_0022 (incoming: 1) {
// stloc V_2(conv ref->i (array.to.pointer(ldloc V_3)))
// ...
// }
// ...
// })
// }
// ...
// } finally BlockContainer {
// Block IL_0043 (incoming: 1) {
// stloc V_3(ldnull)
// leave IL_0043 (nop)
// }
// }
if (!(tryFinally.FinallyBlock is BlockContainer finallyContainer))
return;
if (finallyContainer.Blocks.Count != 1)
return;
var finallyBlock = finallyContainer.Blocks[0];
if (finallyBlock.Instructions[0].MatchStLoc(pinnedRegion.Variable, out var value)
&& value.MatchLdNull()) {
context.Step("Remove store in finally block", finallyBlock);
finallyBlock.Instructions.RemoveAt(0);
}
return;
}
#endregion
}
}

5
ICSharpCode.Decompiler/IL/ILTypeExtensions.cs

@ -136,6 +136,11 @@ namespace ICSharpCode.Decompiler.IL @@ -136,6 +136,11 @@ namespace ICSharpCode.Decompiler.IL
switch (inst) {
case NewObj newObj:
return newObj.Method.DeclaringType;
case NewArr newArr:
if (compilation != null)
return new ArrayType(compilation, newArr.Type, newArr.Indices.Count);
else
return SpecialType.UnknownType;
case Call call:
return call.Method.ReturnType;
case CallVirt callVirt:

Loading…
Cancel
Save