From 887220e771340d3afe4a856f99e08228c41afade Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 23 Apr 2018 16:16:13 -0700 Subject: [PATCH] Fix type error in fixed buffer codegen When generating a pinning temp for fixed statements on fixed buffers, the type for the pinning temp is ref to the type of the fixed field. However, in our lowering the type of a fixed buffer field is a pointer type, while what we actually want for the pinning temp is a ref to the underlying type. This change fixes that scenario. Fixes #26335 --- .../LocalRewriter_FixedStatement.cs | 15 +++- .../Test/Emit/CodeGen/FixedSizeBufferTests.cs | 71 ++++++++++++++++++- .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 4 +- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index dded4032e1b..78c923a2e4a 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -250,9 +250,22 @@ public override BoundNode VisitFixedLocalCollectionInitializer(BoundFixedLocalCo // that is a bit too far // we need to pin the underlying field, and only then take the address. initializerExpr = ((BoundAddressOfOperator)initializerExpr).Operand; - + TypeSymbol initializerType = initializerExpr.Type; + // If this is a fixed buffer, the type is a pointer, but we need + // the underlying type, since we're turning a pointer into a managed ref + if (initializerExpr is BoundFieldAccess fieldAccess && fieldAccess.FieldSymbol.IsFixed) + { + initializerType = ((PointerTypeSymbol)initializerType).PointedAtType; + initializerExpr = fieldAccess.Update( + fieldAccess.ReceiverOpt, + fieldAccess.FieldSymbol, + fieldAccess.ConstantValueOpt, + fieldAccess.ResultKind, + initializerType); + } + // intervening parens may have been skipped by the binder; find the declarator VariableDeclaratorSyntax declarator = fixedInitializer.Syntax.FirstAncestorOrSelf(); Debug.Assert(declarator != null); diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs index e8b41129532..dcbc5c10f2f 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs @@ -14,6 +14,73 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen { public class FixedSizeBufferTests : EmitMetadataTestBase { + [Fact] + public void NestedStructFixed() + { + var verifier = CompileAndVerify(@" +class X { + internal struct CaseRange { + public int Lo, Hi; + public unsafe fixed int Delta [3]; + + public CaseRange (int lo, int hi, int d1, int d2, int d3) + { + Lo = lo; + Hi = hi; + unsafe { + fixed (int *p = Delta) { + p [0] = d1; + p [1] = d2; + p [2] = d3; + } + } + } + } + + static void Main () + { + var a = new CaseRange (0, 0, 0, 0, 0); + } +}", options: TestOptions.UnsafeReleaseExe, verify: Verification.Fails); + verifier.VerifyIL("X.CaseRange..ctor", @" +{ + // Code size 49 (0x31) + .maxstack 3 + .locals init (pinned int& V_0) + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld ""int X.CaseRange.Lo"" + IL_0007: ldarg.0 + IL_0008: ldarg.2 + IL_0009: stfld ""int X.CaseRange.Hi"" + IL_000e: ldarg.0 + IL_000f: ldflda ""int* X.CaseRange.Delta"" + IL_0014: ldflda ""int X.CaseRange.e__FixedBuffer.FixedElementField"" + IL_0019: stloc.0 + IL_001a: ldloc.0 + IL_001b: conv.u + IL_001c: dup + IL_001d: ldarg.3 + IL_001e: stind.i4 + IL_001f: dup + IL_0020: ldc.i4.4 + IL_0021: add + IL_0022: ldarg.s V_4 + IL_0024: stind.i4 + IL_0025: ldc.i4.2 + IL_0026: conv.i + IL_0027: ldc.i4.4 + IL_0028: mul + IL_0029: add + IL_002a: ldarg.s V_5 + IL_002c: stind.i4 + IL_002d: ldc.i4.0 + IL_002e: conv.u + IL_002f: stloc.0 + IL_0030: ret +}"); + } + [Fact] public void SimpleFixedBuffer() { @@ -368,7 +435,7 @@ unsafe static void Main() { // Code size 62 (0x3e) .maxstack 4 - .locals init (pinned int*& V_0) + .locals init (pinned int& V_0) IL_0000: newobj ""S1..ctor()"" IL_0005: dup IL_0006: ldflda ""S S1.field"" @@ -435,7 +502,7 @@ unsafe static void Main() // Code size 47 (0x2f) .maxstack 2 .locals init (int* V_0, //p - pinned int*& V_1) + pinned int& V_1) IL_0000: newobj ""C..ctor()"" IL_0005: ldflda ""S C.s"" IL_000a: ldflda ""int* S.x"" diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index 9471da807a6..817eaffed5a 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -10350,7 +10350,7 @@ unsafe public struct FixedStruct { // Code size 20 (0x14) .maxstack 1 - .locals init (pinned char*& V_0) + .locals init (pinned char& V_0) IL_0000: ldarg.0 IL_0001: ldflda ""char* FixedStruct.c"" IL_0006: ldflda ""char FixedStruct.e__FixedBuffer.FixedElementField"" @@ -10403,7 +10403,7 @@ override public string ToString() { // Code size 45 (0x2d) .maxstack 3 - .locals init (pinned char*& V_0) + .locals init (pinned char& V_0) IL_0000: ldarg.0 IL_0001: ldflda ""char* FixedStruct.c"" IL_0006: ldflda ""char FixedStruct.e__FixedBuffer.FixedElementField"" -- GitLab