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.

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

Binder cycle leads to NullReferenceException #63140

Closed
adamperlin opened this issue Aug 2, 2022 · 4 comments · Fixed by #63142
Closed

Binder cycle leads to NullReferenceException #63140

adamperlin opened this issue Aug 2, 2022 · 4 comments · Fixed by #63142
Assignees
Milestone

Comments

@adamperlin
Copy link
Contributor

adamperlin commented Aug 2, 2022

Version Used: Tip of main

Steps to Reproduce:

interface I 
{
    void M<T>(T s);  
}

class C : I
{
    void I.M<T>(scoped T? s) {}
}

See on sharplab. Run with C# Next: ref fields as currently sharplab is broken for main

Expected Behavior:

error CS9048: The 'scoped' modifier can be used for refs and ref struct values only.
error CS0535: 'C' does not implement interface member 'I.M<T>(T)'
error CS0539: 'C.M<T>(T?)' in explicit interface declaration is not found among members of the interface that can be implemented
warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
error CS0453: The type 'T' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'

Actual Behavior:

System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.)
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Collections.Immutable.ImmutableArray`1.get_Length()

Root Cause

TL; DR This is a cyclic dependence issue in the binder. Attempting to resolve the type of generic nullable type parameter such as T? while the interface mapping for its associated method is being built causes a cycle.

The cycle begins when we try to build the [interface mapping(https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/interfaces#1765-interface-mapping) for type C. It is specified in source that C implements the interface I, so to build the interface mapping we must map all members of I to members of C.

So, for each method defined in I (in this case, there is only one: I.M) we must do the following:
- Search through members defined in C and its possible base types to see if they explicity implement the interface method
- We will find the single member I.M which is supposed to be an explicit interface implementation
- This causes us to attempt to materialize the implementation map entry for I.M since on our first pass it will not be cached
- This means that we need to ask the method I.M of C if it has any explicit interface implementations
- To do this, the MethodSymbol for M has to be materialized, which in this case involves constructing/binding its parameters
- The construction of parameters means that we have to check for parameter errors (See ParameterHelpers.ReportParameterErrors.
- Since the type is marked as scoped this causes a particular condition in ReportParameterErrors to trigger, which means that the .Type field of the parameter is evaluted. This means that we have to get the bound type of a generic type parameter (in this case, T?)
- Since the type parameter is nullable, it becomes a LazyNullableTypeParameter since we don't know if it will be a nullable value or a nullable reference. But we need the type now so we force resolution
- This causes us to check if the method has any constraints on its type parameters
- This means we need to check the constraints of the method we are overriding (or implementing in this case)
- This requires that the LazyNullableTypeParamter get its associated overriden method (See SourceTypeParameterSymbol.OverridenMethod)
- This step needs to compute the explicit implementations for the overriding method. But the field ExplicitImplementations of the overriding method is exactly what we are already trying to compute, so it is null.

> Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ExplicitInterfaceMethodTypeParameterMap.GetOverriddenMethod(Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodSymbol overridingMethod) Line 829        C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.OverriddenMethodTypeParameterMapBase.OverriddenMethod.get() Line 788                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.OverriddenMethodTypeParameterMapBase.GetOverriddenTypeParameter(int ordinal) Line 755               C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOverridingMethodTypeParameterSymbol.OverriddenTypeParameter.get() Line 978                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOverridingMethodTypeParameterSymbol.HasValueTypeConstraint.get() Line 884                C#
   Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol.IsValueType.get() Line 580                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.LazyNullableTypeParameter.GetResolvedType() Line 945                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.LazyNullableTypeParameter.GetResolvedType(Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol defaultType) Line 969     C#
               Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.Type.get() Line 246                C#
               Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ParameterSymbol.Type.get() Line 59           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ParameterHelpers.ReportParameterErrors(Microsoft.CodeAnalysis.CSharp.Symbol owner, Microsoft.CodeAnalysis.CSharp.Syntax.BaseParameterSyntax parameterSyntax, Microsoft.CodeAnalysis.CSharp.Symbols.ParameterSymbol parameter, Microsoft.CodeAnalysis.SyntaxToken thisKeyword, Microsoft.CodeAnalysis.SyntaxToken paramsKeyword, int firstDefault, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 659           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ParameterHelpers.MakeParameters<Microsoft.CodeAnalysis.CSharp.Syntax.ParameterSyntax, Microsoft.CodeAnalysis.CSharp.Symbols.ParameterSymbol, Microsoft.CodeAnalysis.CSharp.Symbol>(Microsoft.CodeAnalysis.CSharp.Binder withTypeParametersBinder, Microsoft.CodeAnalysis.CSharp.Symbol owner, Microsoft.CodeAnalysis.SeparatedSyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.ParameterSyntax> parametersList, out Microsoft.CodeAnalysis.SyntaxToken arglistToken, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, bool allowRefOrOut, bool allowThis, bool addRefReadOnlyModifier, bool suppressUseSiteDiagnostics, int lastIndex, System.Func<Microsoft.CodeAnalysis.CSharp.Binder, Microsoft.CodeAnalysis.CSharp.Symbol, Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations, Microsoft.CodeAnalysis.CSharp.Syntax.ParameterSyntax, Microsoft.CodeAnalysis.RefKind, int, Microsoft.CodeAnalysis.SyntaxToken, Microsoft.CodeAnalysis.SyntaxToken, bool, Microsoft.CodeAnalysis.CSharp.Symbols.DeclarationScope, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag, Microsoft.CodeAnalysis.CSharp.Symbols.ParameterSymbol> parameterCreationFunc, bool parsingFunctionPointer) Line 198                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ParameterHelpers.MakeParameters(Microsoft.CodeAnalysis.CSharp.Binder withTypeParametersBinder, Microsoft.CodeAnalysis.CSharp.Symbol owner, Microsoft.CodeAnalysis.CSharp.Syntax.BaseParameterListSyntax syntax, out Microsoft.CodeAnalysis.SyntaxToken arglistToken, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, bool allowRefOrOut, bool allowThis, bool addRefReadOnlyModifier) Line 30   C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodSymbol.MakeParametersAndBindReturnType(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 135       C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodSymbolBase.MethodChecks(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 84         C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberMethodSymbol.LazyMethodChecks() Line 357                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodOrUserDefinedOperatorSymbol.ExplicitInterfaceImplementations.get() Line 213                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SymbolExtensions.GetExplicitInterfaceImplementations(Microsoft.CodeAnalysis.CSharp.Symbol member) Line 588            C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.MakeExplicitInterfaceImplementationMap() Line 2228             C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.GetExplicitImplementationForInterfaceMember(Microsoft.CodeAnalysis.CSharp.Symbol interfaceMember) Line 2217       C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.ComputeImplementationForInterfaceMember(Microsoft.CodeAnalysis.CSharp.Symbol interfaceMember, Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol implementingType, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, bool ignoreImplementationInInterfaces, out bool implementationInInterfacesMightChangeResult) Line 845              C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.ComputeImplementationAndDiagnosticsForInterfaceMember(Microsoft.CodeAnalysis.CSharp.Symbol interfaceMember, bool ignoreImplementationInInterfaces, out bool implementationInInterfacesMightChangeResult) Line 778                C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.FindImplementationForInterfaceMemberInNonInterfaceWithDiagnostics(Microsoft.CodeAnalysis.CSharp.Symbol interfaceMember, bool ignoreImplementationInInterfacesIfResultIsNotReady) Line 754   C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.ComputeInterfaceImplementations(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, System.Threading.CancellationToken cancellationToken) Line 175     C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.GetSynthesizedExplicitImplementations(System.Threading.CancellationToken cancellationToken) Line 52           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.ForceComplete(Microsoft.CodeAnalysis.SourceLocation locationOpt, System.Threading.CancellationToken cancellationToken) Line 584    C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbol.ForceCompleteMemberByLocation(Microsoft.CodeAnalysis.SourceLocation locationOpt, Microsoft.CodeAnalysis.CSharp.Symbol member, System.Threading.CancellationToken cancellationToken) Line 828         C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceNamespaceSymbol.ForceComplete(Microsoft.CodeAnalysis.SourceLocation locationOpt, System.Threading.CancellationToken cancellationToken) Line 75      C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceModuleSymbol.ForceComplete(Microsoft.CodeAnalysis.SourceLocation locationOpt, System.Threading.CancellationToken cancellationToken) Line 267    C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceAssemblySymbol.ForceComplete(Microsoft.CodeAnalysis.SourceLocation locationOpt, System.Threading.CancellationToken cancellationToken) Line 921    C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetSourceDeclarationDiagnostics(Microsoft.CodeAnalysis.SyntaxTree syntaxTree, Microsoft.CodeAnalysis.Text.TextSpan? filterSpanWithinTree, System.Func<System.Collections.Generic.IEnumerable<Microsoft.CodeAnalysis.Diagnostic>, Microsoft.CodeAnalysis.SyntaxTree, Microsoft.CodeAnalysis.Text.TextSpan?, System.Collections.Generic.IEnumerable<Microsoft.CodeAnalysis.Diagnostic>> locationFilterOpt, System.Threading.CancellationToken cancellationToken) Line 2951           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnosticsWithoutFiltering(Microsoft.CodeAnalysis.CompilationStage stage, bool includeEarlierStages, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag builder, System.Threading.CancellationToken cancellationToken) Line 2721           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(Microsoft.CodeAnalysis.CompilationStage stage, bool includeEarlierStages, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, System.Threading.CancellationToken cancellationToken) Line 2647           C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(Microsoft.CodeAnalysis.CompilationStage stage, bool includeEarlierStages, System.Threading.CancellationToken cancellationToken) Line 2639          C#
Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnostics(System.Threading.CancellationToken cancellationToken) Line 2633       C#
Microsoft.CodeAnalysis.Test.Utilities.dll!Microsoft.CodeAnalysis.DiagnosticExtensions.VerifyDiagnostics<Microsoft.CodeAnalysis.CSharp.CSharpCompilation>(Microsoft.CodeAnalysis.CSharp.CSharpCompilation c, Microsoft.CodeAnalysis.Test.Utilities.DiagnosticDescription[] expected) Line 107            C#
Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.dll!Microsoft.CodeAnalysis.CSharp.UnitTests.Semantics.NullableReferenceTypesTests.SimpleGenericNullableTest() Line 106379         C#
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 2, 2022
@jcouv jcouv removed their assignment Aug 2, 2022
@cston
Copy link
Member

cston commented Aug 2, 2022

It looks like this issue is fixed in #62783. I'll add a test there, thanks.

Never mind, it still repros there.

@jcouv
Copy link
Member

jcouv commented Aug 2, 2022

@cston The issue is general (not specific to scoped). Any logic that pulls on parameter.Type in ReportParameterErrors risks hitting this cycle.
The scoped check is the only one we can repro at the moment, but trying to call ReportParameterErrors from lambda binding logic would also make this issue salient.

@cston
Copy link
Member

cston commented Aug 2, 2022

Any logic that pulls on parameter.Type in ReportParameterErrors risks hitting this cycle.

True, but if the only use is for parameter.Type.IsRefLikeType, we could handle that one case. (There was a similar issue in #62778 which could be resolved by handling IsRefLikeType directly.)

@cston
Copy link
Member

cston commented Aug 2, 2022

We could potentially handle this by adding a TypeWithAnnotations.IsRefLikeType() method that avoids resolving type parameters. (See TypeWithAnnotations.IsSZArray() for similar behavior.)

@jcouv jcouv assigned cston and unassigned 333fred and adamperlin Aug 2, 2022
@jcouv jcouv added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 2, 2022
@jcouv jcouv added this to the 17.4 milestone Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants