Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suppress dynamic access to instance indexers off of types. #11677

Merged
merged 1 commit into from
Jun 1, 2016
Merged

Suppress dynamic access to instance indexers off of types. #11677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6041,9 +6041,13 @@ private BoundExpression BindIndexerOrIndexedPropertyAccess(

if (analyzedArguments.HasDynamicArgument && overloadResolutionResult.HasAnyApplicableMember)
{
var result = BindDynamicIndexer(syntax, receiverOpt, analyzedArguments, overloadResolutionResult.GetAllApplicableMembers(), diagnostics);
// Note that the runtime binder may consider candidates that haven't passed compile-time final validation
// and an ambiguity error may be reported. Also additional checks are performed in runtime final validation
// that are not performed at compile-time.
// Only if the set of final applicable candidates is empty we know for sure the call will fail at runtime.
var finalApplicableCandidates = GetCandidatesPassingFinalValidation(syntax, overloadResolutionResult, receiverOpt, default(ImmutableArray<TypeSymbol>), diagnostics);
overloadResolutionResult.Free();
return result;
return BindDynamicIndexer(syntax, receiverOpt, analyzedArguments, finalApplicableCandidates, diagnostics);
}

if (!overloadResolutionResult.Succeeded)
Expand Down
18 changes: 13 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,10 @@ private BoundExpression BindMethodGroupInvocation(
// and an ambiguity error may be reported. Also additional checks are performed in runtime final validation
// that are not performed at compile-time.
// Only if the set of final applicable candidates is empty we know for sure the call will fail at runtime.
var finalApplicableCandidates = GetCandidatesPassingFinalValidation(syntax, resolution.OverloadResolutionResult, methodGroup, diagnostics);
var finalApplicableCandidates = GetCandidatesPassingFinalValidation(syntax, resolution.OverloadResolutionResult,
methodGroup.ReceiverOpt,
methodGroup.TypeArgumentsOpt,
diagnostics);
if (finalApplicableCandidates.Length > 0)
{
result = BindDynamicInvocation(syntax, methodGroup, resolution.AnalyzedArguments, finalApplicableCandidates, diagnostics, queryClause);
Expand All @@ -569,11 +572,16 @@ private BoundExpression BindMethodGroupInvocation(
return result;
}

private ImmutableArray<MethodSymbol> GetCandidatesPassingFinalValidation(CSharpSyntaxNode syntax, OverloadResolutionResult<MethodSymbol> overloadResolutionResult, BoundMethodGroup methodGroup, DiagnosticBag diagnostics)
private ImmutableArray<TMethodOrPropertySymbol> GetCandidatesPassingFinalValidation<TMethodOrPropertySymbol>(
CSharpSyntaxNode syntax,
OverloadResolutionResult<TMethodOrPropertySymbol> overloadResolutionResult,
BoundExpression receiverOpt,
ImmutableArray<TypeSymbol> typeArgumentsOpt,
DiagnosticBag diagnostics) where TMethodOrPropertySymbol : Symbol
{
Debug.Assert(overloadResolutionResult.HasAnyApplicableMember);

var finalCandidates = ArrayBuilder<MethodSymbol>.GetInstance();
var finalCandidates = ArrayBuilder<TMethodOrPropertySymbol>.GetInstance();
DiagnosticBag firstFailed = null;
DiagnosticBag candidateDiagnostics = DiagnosticBag.GetInstance();

Expand All @@ -590,8 +598,8 @@ private ImmutableArray<MethodSymbol> GetCandidatesPassingFinalValidation(CSharpS
// * If F is an instance method, the method group must have resulted from a simple-name, a member-access through a variable or value,
// or a member-access whose receiver can't be classified as a type or value until after overload resolution (see §7.6.4.1).

if (!MemberGroupFinalValidationAccessibilityChecks(methodGroup.ReceiverOpt, result.Member, syntax, candidateDiagnostics, invokedAsExtensionMethod: false) &&
(methodGroup.TypeArgumentsOpt.IsDefault || result.Member.CheckConstraints(this.Conversions, syntax, this.Compilation, candidateDiagnostics)))
if (!MemberGroupFinalValidationAccessibilityChecks(receiverOpt, result.Member, syntax, candidateDiagnostics, invokedAsExtensionMethod: false) &&
(typeArgumentsOpt.IsDefault || ((MethodSymbol)(object)result.Member).CheckConstraints(this.Conversions, syntax, this.Compilation, candidateDiagnostics)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the double conversion: (MethodSymbol)(object)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler refused to convert from type parameter directly.

{
finalCandidates.Add(result.Member);
continue;
Expand Down
255 changes: 255 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/DynamicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3550,5 +3550,260 @@ .maxstack 8
Assert.Equal(typeGConstructed, typeD.GetMember<FieldSymbol>("ExtraTrue").Type);
Assert.Equal(typeGConstructed, typeD.GetMember<FieldSymbol>("ExtraFalse").Type);
}

[ClrOnlyFact(ClrOnlyReason.Ilasm)]
[WorkItem(204561, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=204561&_a=edit")]
public void SuppressDynamicIndexerAccessOffOfType_01()
{
var iLSource = @"
.assembly extern mscorlib
{
.publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4..
.ver 4:0:0:0
}

.assembly Microsoft.Office.Interop.Excel
{
.custom instance void [mscorlib]System.Runtime.InteropServices.ImportedFromTypeLibAttribute::.ctor(string) = ( 01 00 05 45 78 63 65 6C 00 00 ) // ...Excel..
.custom instance void [mscorlib]System.Runtime.InteropServices.PrimaryInteropAssemblyAttribute::.ctor(int32,
int32) = ( 01 00 01 00 00 00 08 00 00 00 00 00 )
.custom instance void [mscorlib]System.Runtime.InteropServices.GuidAttribute::.ctor(string) = ( 01 00 24 30 30 30 32 30 38 31 33 2D 30 30 30 30 // ..$00020813-0000
2D 30 30 30 30 2D 63 30 30 30 2D 30 30 30 30 30 // -0000-c000-00000
30 30 30 30 30 34 36 00 00 ) // 0000046..
.custom instance void [mscorlib]System.Runtime.InteropServices.TypeLibVersionAttribute::.ctor(int32,
int32) = ( 01 00 01 00 00 00 08 00 00 00 00 00 )
.hash algorithm 0x00008004
.ver 15:0:0:0
}
.module Excel.dll
// MVID: {C7C599B3-5C80-48BC-9637-7CADFEF6DEB8}
.imagebase 0x00400000
.file alignment 0x00001000
.stackreserve 0x00100000
.subsystem 0x0003 // WINDOWS_CUI
.corflags 0x00000009 // ILONLY
// Image base: 0x050E0000

.class interface public abstract auto ansi import Microsoft.Office.Interop.Excel.Worksheet
implements Microsoft.Office.Interop.Excel._Worksheet
{
.custom instance void [mscorlib]System.Runtime.InteropServices.GuidAttribute::.ctor(string) = ( 01 00 24 30 30 30 32 30 38 44 38 2D 30 30 30 30 // ..$000208D8-0000
2D 30 30 30 30 2D 43 30 30 30 2D 30 30 30 30 30 // -0000-C000-00000
30 30 30 30 30 34 36 00 00 ) // 0000046..
} // end of class Microsoft.Office.Interop.Excel.Worksheet

.class interface public abstract auto ansi import Microsoft.Office.Interop.Excel._Worksheet
{
.custom instance void [mscorlib]System.Runtime.InteropServices.TypeLibTypeAttribute::.ctor(int16) = ( 01 00 C0 10 00 00 )
.custom instance void [mscorlib]System.Runtime.InteropServices.GuidAttribute::.ctor(string) = ( 01 00 24 30 30 30 32 30 38 44 38 2D 30 30 30 30 // ..$000208D8-0000
2D 30 30 30 30 2D 43 30 30 30 2D 30 30 30 30 30 // -0000-C000-00000
30 30 30 30 30 34 36 00 00 ) // 0000046..

.method public hidebysig newslot specialname abstract virtual
instance class Microsoft.Office.Interop.Excel.Range
marshal( interface )
get_Range([in] object marshal( struct) Cell1,
[in][opt] object marshal( struct) Cell2) runtime managed internalcall
{
.custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 C5 00 00 00 00 00 )
} // end of method _Worksheet::get_Range

.property class Microsoft.Office.Interop.Excel.Range
Range(object,
object)
{
.custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 C5 00 00 00 00 00 )
.get instance class Microsoft.Office.Interop.Excel.Range Microsoft.Office.Interop.Excel._Worksheet::get_Range(object,
object)
} // end of property _Worksheet::Range

.method public hidebysig newslot specialname abstract virtual
instance class Microsoft.Office.Interop.Excel.Range
marshal( interface )
MRange([in] object marshal( struct) Cell1,
[in][opt] object marshal( struct) Cell2) runtime managed internalcall
{
.custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 C5 00 00 00 00 00 )
} // end of method _Worksheet::get_Range
}

.class interface public abstract auto ansi import Microsoft.Office.Interop.Excel.Range
implements [mscorlib]System.Collections.IEnumerable
{
.custom instance void [mscorlib]System.Runtime.InteropServices.InterfaceTypeAttribute::.ctor(int16) = ( 01 00 02 00 00 00 )
.custom instance void [mscorlib]System.Reflection.DefaultMemberAttribute::.ctor(string) = ( 01 00 08 5F 44 65 66 61 75 6C 74 00 00 ) // ..._Default..
.custom instance void [mscorlib]System.Runtime.InteropServices.GuidAttribute::.ctor(string) = ( 01 00 24 30 30 30 32 30 38 34 36 2D 30 30 30 30 // ..$00020846-0000
2D 30 30 30 30 2D 43 30 30 30 2D 30 30 30 30 30 // -0000-C000-00000
30 30 30 30 30 34 36 00 00 ) // 0000046..
.custom instance void [mscorlib]System.Runtime.InteropServices.TypeLibTypeAttribute::.ctor(int16) = ( 01 00 00 10 00 00 )
}
";

MetadataReference reference = CompileIL(iLSource, appendDefaultHeader: false, embedInteropTypes: false);

string consumer1 = @"
using Microsoft.Office.Interop.Excel;

class Test
{
public static void Main()
{
dynamic x = 1;
dynamic y = 1;

var z2 = Worksheet.MRange(x, y);
}
}
";

var compilation1 = CreateCompilationWithMscorlib(consumer1, options: TestOptions.ReleaseExe,
references: new MetadataReference[] { reference, CSharpRef, SystemCoreRef });

compilation1.VerifyDiagnostics(
// (11,18): error CS0120: An object reference is required for the non-static field, method, or property '_Worksheet.MRange(object, object)'
// var z2 = Worksheet.MRange(x, y);
Diagnostic(ErrorCode.ERR_ObjectRequired, "Worksheet.MRange(x, y)").WithArguments("Microsoft.Office.Interop.Excel._Worksheet.MRange(object, object)").WithLocation(11, 18)
);

string consumer2 = @"
using Microsoft.Office.Interop.Excel;

class Test
{
public static void Main()
{
dynamic x = 1;
dynamic y = 1;
var z1 = Worksheet.Range[x, y];
}
}
";

var compilation2 = CreateCompilationWithMscorlib(consumer2, options: TestOptions.ReleaseExe,
references: new MetadataReference[] { reference, CSharpRef, SystemCoreRef });

compilation2.VerifyDiagnostics(
// (10,18): error CS0120: An object reference is required for the non-static field, method, or property '_Worksheet.Range[object, object]'
// var z1 = Worksheet.Range[x, y];
Diagnostic(ErrorCode.ERR_ObjectRequired, "Worksheet.Range[x, y]").WithArguments("Microsoft.Office.Interop.Excel._Worksheet.Range[object, object]").WithLocation(10, 18)
);
}

[ClrOnlyFact(ClrOnlyReason.Ilasm)]
[WorkItem(204561, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=204561&_a=edit")]
public void SuppressDynamicIndexerAccessOffOfType_02()
{
var iLSource = @"
.class public auto ansi WithIndexer
extends [mscorlib]System.Object
{
.method public specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
} // end of method WithIndexer::.ctor

.method public specialname static object
get_Indexer(object x,
object y) cil managed
{
// Code size 18 (0x12)
.maxstack 1
.locals init (object V_0)
IL_0000: nop
IL_0001: ldstr ""Indexer""
IL_0006: call void [mscorlib]System.Console::WriteLine(string)
IL_000b: nop
IL_000c: ldnull
IL_000d: stloc.0
IL_000e: br.s IL_0010

IL_0010: ldloc.0
IL_0011: ret
} // end of method WithIndexer::get_Indexer

.method public specialname static void
set_Indexer(object x,
object y,
object 'value') cil managed
{
// Code size 2 (0x2)
.maxstack 8
IL_0000: nop
IL_0001: ret
} // end of method WithIndexer::set_Indexer

.method public static object MIndexer(object x,
object y) cil managed
{
// Code size 18 (0x12)
.maxstack 1
.locals init (object V_0)
IL_0000: nop
IL_0001: ldstr ""MIndexer""
IL_0006: call void [mscorlib]System.Console::WriteLine(string)
IL_000b: nop
IL_000c: ldnull
IL_000d: stloc.0
IL_000e: br.s IL_0010

IL_0010: ldloc.0
IL_0011: ret
} // end of method WithIndexer::MIndexer

.property object Indexer(object,
object)
{
.get object WithIndexer::get_Indexer(object,
object)
.set void WithIndexer::set_Indexer(object,
object,
object)
} // end of property WithIndexer::Indexer
} // end of class WithIndexer
";

MetadataReference reference = CompileIL(iLSource, appendDefaultHeader: true, embedInteropTypes: false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can an assembly of this structure only be created in IL? Just asking because if it could also be created in VB it may be interesting to have a test with VB code to ensure reasonable interop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to test from IL in this case. As you can see, there is no interop with indexed property in this case and that is the scenario we are interested about.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I suppose producing a binding error isn't interop. Sounds good.


string consumer1 = @"
class Test
{
public static void Main()
{
dynamic x = 1;
dynamic y = 1;
var z2 = WithIndexer.MIndexer(x, y);
}
}";

var compilation1 = CreateCompilationWithMscorlib(consumer1, options: TestOptions.ReleaseExe,
references: new MetadataReference[] { reference, CSharpRef, SystemCoreRef });

CompileAndVerify(compilation1, expectedOutput: "MIndexer").VerifyDiagnostics();

string consumer2 = @"
class Test
{
public static void Main()
{
dynamic x = 1;
dynamic y = 1;
var z1 = WithIndexer.Indexer[x, y];
}
}";

var compilation2 = CreateCompilationWithMscorlib(consumer2, options: TestOptions.ReleaseExe,
references: new MetadataReference[] { reference, CSharpRef, SystemCoreRef });

compilation2.VerifyDiagnostics(
// (8,30): error CS1545: Property, indexer, or event 'WithIndexer.Indexer[object, object]' is not supported by the language; try directly calling accessor methods 'WithIndexer.get_Indexer(object, object)' or 'WithIndexer.set_Indexer(object, object, object)'
// var z1 = WithIndexer.Indexer[x, y];
Diagnostic(ErrorCode.ERR_BindToBogusProp2, "Indexer").WithArguments("WithIndexer.Indexer[object, object]", "WithIndexer.get_Indexer(object, object)", "WithIndexer.set_Indexer(object, object, object)").WithLocation(8, 30)
);
}
}
}