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

NRE at Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey`1.<EnumerateSymbols> #11092

Closed
mavasani opened this issue May 5, 2016 · 11 comments
Labels
Area-IDE Bug Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. Urgency-Soon
Milestone

Comments

@mavasani
Copy link
Contributor

mavasani commented May 5, 2016

I do not have a repro, but am hitting this NRE very often while typing. It seems to be from RenameTrackingTaggerProvider.

I have InstallDogfood bits from master which are a day old (1.03.0.60503)

Dump saved at: \\<%Internal_Share%>\public\dumps\11092\devenv.dmp

Message: Object reference not set to an instance of an object.

Call stack:

at Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey1.<EnumerateSymbols>d__5.MoveNext() at Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey1..ctor(TSymbol symbol, Visitor visitor)
at Microsoft.CodeAnalysis.SymbolKey.Visitor.VisitLocal(ILocalSymbol localSymbol)
at Microsoft.CodeAnalysis.CSharp.Symbols.LocalSymbol.Accept[TResult](SymbolVisitor`1 visitor)
at Microsoft.CodeAnalysis.SymbolKey.GetOrCreate(ISymbol symbol, Visitor visitor)
at Microsoft.CodeAnalysis.FindSymbols.SymbolFinder.d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking.RenameTrackingTaggerProvider.TrackingSession.d__22.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
at Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking.RenameTrackingTaggerProvider.TrackingSession.d__20.MoveNext()

@mavasani mavasani added Bug Area-IDE Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. labels May 5, 2016
@mavasani mavasani added this to the 1.3 milestone May 5, 2016
@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2016

I am hitting this frequently even with the latest InstallDogfood bits from master - I am marking this as urgency soon and rolling back to older Roslyn bits :-(

@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2016

FYI @dpoeschl

@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2016

Note that all the crashes seem to occur when there are errors in code being edited. I rolled back to Roslyn bits from 27th April and still hitting this crash. Rename tracking diagnostic is marked as NotConfigurable, so I guess it cannot be suppressed either.

@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2016

I rolled back to bits from the 26th April and haven't seen any more crashes. Probably something checked in during these 2 days that regressed this?

@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2016

Commented too soon - got the same exception on bits from 26th as well :-(

@jmarolf
Copy link
Contributor

jmarolf commented May 6, 2016

@mavasani ran into this just now.

   at Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey`1.<EnumerateSymbols>d__5.MoveNext()
   at Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey`1..ctor(TSymbol symbol, Visitor visitor)
   at Microsoft.CodeAnalysis.SymbolKey.Visitor.VisitLocal(ILocalSymbol localSymbol)
   at Microsoft.CodeAnalysis.CSharp.Symbols.LocalSymbol.Accept[TResult](SymbolVisitor`1 visitor)
   at Microsoft.CodeAnalysis.SymbolKey.GetOrCreate(ISymbol symbol, Visitor visitor)
   at Microsoft.CodeAnalysis.FindSymbols.SymbolFinder.<FindSourceDefinitionWorkerAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking.RenameTrackingTaggerProvider.TrackingSession.<DetermineIfRenamableSymbolAsync>d__22.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking.RenameTrackingTaggerProvider.TrackingSession.<DetermineIfRenamableIdentifierAsync>d__20.MoveNext()

Looks to be the same issue. I'll take a look.

@mavasani
Copy link
Contributor Author

mavasani commented May 9, 2016

I have a consistent repro now:

  1. Git clone roslyn-analyzers git repo and checkout the sources from 8th May
  2. cibuild - to build the repo
  3. Open src\Analyzers.sln
  4. Open file: \src\Microsoft.QualityGuidelines.Analyzers\Core\UseLiteralsWhereAppropriate.cs
  5. Go to line 73 and delete all text after 'fieldInitializerValue.Type' on that line in the if condition, including the && operator.
  6. Enter '.SpecialType ==' after 'fieldInitializerValue.Type' and hit space - you should hit this crash

@mavasani
Copy link
Contributor Author

mavasani commented May 9, 2016

Can we also consider making Rename tracking diagnostic configurable, so that it can be disabled if the user doesn't care about this feature?

@jmarolf
Copy link
Contributor

jmarolf commented May 9, 2016

@mavasani 's repro steps uncover a different crash. Filed as #11193

@jmarolf
Copy link
Contributor

jmarolf commented May 11, 2016

So here's whats happening:

public static SymbolKey GetSymbolKey(this ISymbol symbol)
{
    return SymbolKey.Create(symbol, null, CancellationToken.None);
}

We call symbol.GetSymbolKey() which calls SymbolKey.Create passing in a null compilation.

internal static SymbolKey Create(ISymbol symbol, CancellationToken cancellationToken = default(CancellationToken))
{
    return GetOrCreate(symbol, new Visitor(compilation, cancellationToken));
}

We create a new visitor object with a null compilation. Normally this would be the end of it but we crash due to this null compilation later.

private static SymbolKey GetOrCreate(ISymbol symbol, Visitor visitor)
{
    if (symbol == null)
    {
        return s_null;
    }

    SymbolKey result;
    if (!visitor.SymbolCache.TryGetValue(symbol, out result))
    {
>>      result = symbol.Accept(visitor);
        visitor.SymbolCache[symbol] = result;
    }

    return result;
}

The call to Accept will cause the visitor to be exercised. We jump through the visit methods until we arrive at

public override SymbolKey VisitLocal(ILocalSymbol localSymbol)
{
>>  return new NonDeclarationSymbolKey<ILocalSymbol>(localSymbol, this);
}

Which calls Enumerate Symbols

internal NonDeclarationSymbolKey(TSymbol symbol, Visitor visitor)
{
    var containingSymbol = symbol.ContainingSymbol;

    while (!containingSymbol.DeclaringSyntaxReferences.Any())
    {
        containingSymbol = containingSymbol.ContainingSymbol;
    }

    _containingKey = GetOrCreate(containingSymbol, visitor);
    _localName = symbol.Name;

>>  foreach (var possibleSymbol in EnumerateSymbols(visitor.Compilation, containingSymbol, _localName, visitor.CancellationToken))
    {
        if (possibleSymbol.Item1.Equals(symbol))
        {
            _ordinal = possibleSymbol.Item2;
            break;
        }
    }
}

In this case because of the visitation pattern we end up calling EnumerateSymbols and passing in a null compilation.
This will always crash if we end of visiting a local as part of creating a SymbolKey

@mavasani
Copy link
Contributor Author

mavasani commented May 11, 2016

@jmarolf I have consistent repro for this now, and I verified that this time the exception is NRE from Microsoft.CodeAnalysis.SymbolKey.NonDeclarationSymbolKey

  1. Checkout Roslyn code from my PR: Ensure that we suppress compilation diagnostics reported in generated… #11225 (commit 6f90c89) - I hope to merge this PR soon, so you may be able to skip this step.
  2. Open Roslyn.sln and wait for solution load to complete.
  3. Open file src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs and close all other documents.
  4. Goto line 657:
    (if (!_lazyGeneratedCodeSymbolsMap.TryGetValue(tree, out existingGeneratedCodeSymbols)))
  5. Save All and close the solution. Restart VS and open Roslyn.sln again - wait for solution load to complete.
  6. Put the cursor at the end of existingGeneratedCodeSymbols, line 657, column 101 and type any character - you should hit this crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Tenet-Reliability Customer telemetry indicates that the product is failing in a crash/hang/dataloss manner. Urgency-Soon
Projects
None yet
Development

No branches or pull requests

2 participants