diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index 79a5cc00de7fa..b7ac2374dfa94 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -6153,6 +6153,16 @@ private static void CombineExtensionMethodArguments(BoundExpression receiver, An if (!hasError && fieldSymbol.IsFixed && !IsInsideNameof) { + // SPEC: In a member access of the form E.I, if E is of a struct type and a member lookup of I in + // that struct type identifies a fixed size member, then E.I is evaluated an classified as follows: + // * If the expression E.I does not occur in an unsafe context, a compile-time error occurs. + // * If E is classified as a value, a compile-time error occurs. + // * Otherwise, if E is a moveable variable and the expression E.I is not a fixed_pointer_initializer, + // a compile-time error occurs. + // * Otherwise, E references a fixed variable and the result of the expression is a pointer to the + // first element of the fixed size buffer member I in E. The result is of type S*, where S is + // the element type of I, and is classified as a value. + TypeSymbol receiverType = receiver.Type; // Reflect errors that have been reported elsewhere... @@ -6161,11 +6171,13 @@ private static void CombineExtensionMethodArguments(BoundExpression receiver, An if (!hasError) { var isFixedStatementExpression = SyntaxFacts.IsFixedStatementExpression(node); - Symbol accessedLocalOrParameterOpt; - if (ExpressionRequiresFixing(receiver, out accessedLocalOrParameterOpt) != isFixedStatementExpression) + + if (IsMoveableVariable(receiver, out Symbol accessedLocalOrParameterOpt) != isFixedStatementExpression) { if (indexed) { + // SPEC C# 7.3: If the fixed size buffer access is the receiver of an element_access_expression, + // E may be either fixed or moveable CheckFeatureAvailability(node, MessageID.IDS_FeatureIndexingMovableFixedBuffers, diagnostics); } else diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs index ce764b7ba76f0..ecf2a079e1674 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs @@ -2120,7 +2120,7 @@ private BoundExpression BindAddressOfExpression(PrefixUnaryExpressionSyntax node if (!hasErrors) { Symbol accessedLocalOrParameterOpt; - if (ExpressionRequiresFixing(operand, out accessedLocalOrParameterOpt) != isFixedStatementAddressOfExpression) + if (IsMoveableVariable(operand, out accessedLocalOrParameterOpt) != isFixedStatementAddressOfExpression) { Error(diagnostics, isFixedStatementAddressOfExpression ? ErrorCode.ERR_FixedNotNeeded : ErrorCode.ERR_FixedNeeded, node); hasErrors = true; @@ -2135,9 +2135,13 @@ private BoundExpression BindAddressOfExpression(PrefixUnaryExpressionSyntax node return new BoundAddressOfOperator(node, operand, pointerType, hasErrors); } - // Basically a port of ExpressionBinder::isFixedExpression, which basically implements spec section 18.3. - // NOTE: internal to allow using in tests. - internal bool ExpressionRequiresFixing(BoundExpression expr, out Symbol accessedLocalOrParameterOpt) + /// + /// Checks to see whether an expression is a "moveable" variable according to the spec. Moveable + /// variables have underlying memory which may be moved by the runtime. The spec defines anything + /// not fixed as moveable and specifies the expressions which are fixed. + /// + + internal bool IsMoveableVariable(BoundExpression expr, out Symbol accessedLocalOrParameterOpt) { accessedLocalOrParameterOpt = null; @@ -2233,22 +2237,21 @@ internal bool ExpressionRequiresFixing(BoundExpression expr, out Symbol accessed } case BoundKind.PointerElementAccess: { - // When using pointers in unsafe code, it is generally left for the user to make sure memory is fixed/pinned correctly. - // Except for fixed fields, we want to produce appropriate diagnostics if field is not fixed before access. + // C# 7.3: + // a variable resulting from a... pointer_element_access of the form P[E] [is fixed] if P + // is not a fixed size buffer expression, or if the expression is a fixed size buffer + // member_access of the form E.I and E is a fixed variable BoundExpression underlyingExpr = ((BoundPointerElementAccess)expr).Expression; - if (underlyingExpr.Kind == BoundKind.FieldAccess) + if (underlyingExpr is BoundFieldAccess fieldAccess && fieldAccess.FieldSymbol.IsFixed) { - FieldSymbol field = ((BoundFieldAccess)underlyingExpr).FieldSymbol; - if (field.IsFixed) - { - return true; - } + expr = fieldAccess.ReceiverOpt; + continue; } return false; } - case BoundKind.PropertyAccess: // Never a variable. - case BoundKind.IndexerAccess: // Never a variable. + case BoundKind.PropertyAccess: // Never fixed + case BoundKind.IndexerAccess: // Never fixed default: { return true; diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs index 4b98c661218fb..f27c2dcdd4a26 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/FixedSizeBufferTests.cs @@ -1067,5 +1067,72 @@ unsafe struct Foo // public int* M2 => &Bar[1]; Diagnostic(ErrorCode.ERR_FixedNeeded, "&Bar[1]").WithLocation(7, 23)); } + + [Fact] + public void StaticField() + { + var verifier = CompileAndVerify(@" +unsafe struct S +{ + public fixed int Buf[1]; +} +unsafe class C +{ + static S s_f; + public void M() + { + s_f.Buf[0] = 1; + } +}", options: TestOptions.UnsafeReleaseDll); + verifier.VerifyIL("C.M", @" +{ + // Code size 18 (0x12) + .maxstack 2 + IL_0000: ldsflda ""S C.s_f"" + IL_0005: ldflda ""int* S.Buf"" + IL_000a: ldflda ""int S.e__FixedBuffer.FixedElementField"" + IL_000f: ldc.i4.1 + IL_0010: stind.i4 + IL_0011: ret +}"); + } + + [Fact] + public void FixedSizeBufferOffPointerAcess() + { + var verifier = CompileAndVerify(@" +using System; +unsafe struct S +{ + public fixed int Buf[1]; +} +unsafe class C +{ + public void M(IntPtr ptr) + { + S* s = (S*)ptr; + int* x = s->Buf; + int* y = &s->Buf[0]; + } +}", options: TestOptions.UnsafeReleaseDll, verify: Verification.Fails); + verifier.VerifyIL("C.M", @" +{ + // Code size 32 (0x20) + .maxstack 1 + .locals init (S* V_0) //s + IL_0000: ldarg.1 + IL_0001: call ""void* System.IntPtr.op_Explicit(System.IntPtr)"" + IL_0006: stloc.0 + IL_0007: ldloc.0 + IL_0008: ldflda ""int* S.Buf"" + IL_000d: ldflda ""int S.e__FixedBuffer.FixedElementField"" + IL_0012: pop + IL_0013: ldloc.0 + IL_0014: ldflda ""int* S.Buf"" + IL_0019: ldflda ""int S.e__FixedBuffer.FixedElementField"" + IL_001e: pop + IL_001f: ret +}"); + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs index dc700adbfc123..93a11fb55b2df 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs @@ -2308,7 +2308,7 @@ public override BoundNode Visit(BoundNode node) { text = SymbolDisplay.FormatLiteral(text, quote: false); - if (_binder.ExpressionRequiresFixing(expr, out Symbol accessedLocalOrParameterOpt)) + if (_binder.IsMoveableVariable(expr, out Symbol accessedLocalOrParameterOpt)) { _builder.Add($"Yes, {expr.Kind} '{text}' requires fixing."); } @@ -8690,5 +8690,123 @@ unsafe void M(in int p) } #endregion + + [Fact] + public void AddressOfFixedSizeBuffer() + { + CreateCompilation(@" +unsafe struct S +{ + public fixed int Buf[1]; +} + +unsafe class C +{ + static S s_f; + void M(S s) + { + fixed (int* a = &s.Buf) {} + fixed (int* b = &s_f.Buf) {} + int* c = &s.Buf; + int* d = &s_f.Buf; + } +}", options: TestOptions.UnsafeDebugDll).VerifyDiagnostics( + // (12,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // fixed (int* a = &s.Buf) {} + Diagnostic(ErrorCode.ERR_FixedNotNeeded, "&s.Buf").WithLocation(12, 25), + // (13,26): error CS1666: You cannot use fixed size buffers contained in unfixed expressions. Try using the fixed statement. + // fixed (int* b = &s_f.Buf) {} + Diagnostic(ErrorCode.ERR_FixedBufferNotFixed, "s_f.Buf").WithLocation(13, 26), + // (14,18): error CS0266: Cannot implicitly convert type 'int**' to 'int*'. An explicit conversion exists (are you missing a cast?) + // int* c = &s.Buf; + Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "&s.Buf").WithArguments("int**", "int*").WithLocation(14, 18), + // (15,19): error CS1666: You cannot use fixed size buffers contained in unfixed expressions. Try using the fixed statement. + // int* d = &s_f.Buf; + Diagnostic(ErrorCode.ERR_FixedBufferNotFixed, "s_f.Buf").WithLocation(15, 19)); + } + + [Fact] + public void FixedFixedSizeBuffer() + { + CreateCompilation(@" +unsafe struct S +{ + public fixed int Buf[1]; +} + +unsafe class C +{ + static S s_f; + void M(S s) + { + fixed (int* a = s.Buf) {} + fixed (int* b = s_f.Buf) {} + int* c = s.Buf; + int* d = s_f.Buf; + } +}", options: TestOptions.UnsafeDebugDll).VerifyDiagnostics( + // (12,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // fixed (int* a = s.Buf) {} + Diagnostic(ErrorCode.ERR_FixedNotNeeded, "s.Buf").WithLocation(12, 25), + // (15,18): error CS1666: You cannot use fixed size buffers contained in unfixed expressions. Try using the fixed statement. + // int* d = s_f.Buf; + Diagnostic(ErrorCode.ERR_FixedBufferNotFixed, "s_f.Buf").WithLocation(15, 18)); + } + + [Fact] + public void NoPointerDerefMoveableFixedSizeBuffer() + { + CreateCompilation(@" +unsafe struct S +{ + public fixed int Buf[1]; +} + +unsafe class C +{ + static S s_f; + void M(S s) + { + int x = *s.Buf; + int y = *s_f.Buf; + } +}", options: TestOptions.UnsafeDebugDll).VerifyDiagnostics( + // (13,18): error CS1666: You cannot use fixed size buffers contained in unfixed expressions. Try using the fixed statement. + // int y = *s_f.Buf; + Diagnostic(ErrorCode.ERR_FixedBufferNotFixed, "s_f.Buf").WithLocation(13, 18)); + } + + [Fact] + public void AddressOfElementAccessFixedSizeBuffer() + { + CreateCompilation(@" +unsafe struct S +{ + public fixed int Buf[1]; +} + +unsafe class C +{ + static S s_f; + void M(S s) + { + fixed (int* a = &s.Buf[0]) { } + fixed (int* b = &s_f.Buf[0]) { } + int* c = &s.Buf[0]; + int* d = &s_f.Buf[0]; + int* e = &(s.Buf[0]); + int* f = &(s_f.Buf[0]); + } +}", options: TestOptions.UnsafeDebugDll).VerifyDiagnostics( + // (12,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // fixed (int* a = &s.Buf[0]) { } + Diagnostic(ErrorCode.ERR_FixedNotNeeded, "&s.Buf[0]").WithLocation(12, 25), + // (15,18): error CS0212: You can only take the address of an unfixed expression inside of a fixed statement initializer + // int* d = &s_f.Buf[0]; + Diagnostic(ErrorCode.ERR_FixedNeeded, "&s_f.Buf[0]").WithLocation(15, 18), + // (17,18): error CS0212: You can only take the address of an unfixed expression inside of a fixed statement initializer + // int* f = &(s_f.Buf[0]); + Diagnostic(ErrorCode.ERR_FixedNeeded, "&(s_f.Buf[0])").WithLocation(17, 18)); + } } }