ISymbol.ToDisplayString throws NullReferenceException in SymbolDisplayVisitor

[pavel-mikula-sonarsource]

Version Used:
6.0.100-preview.3.21202.5 in our CI
5.0.300 as reported by @runehalfdan in our repo SonarSource/sonar-dotnet#4525 (comment)

Steps to Reproduce:

This issue doesn't reproduce in a stable way. There's some nondeterminism in the build and re-running the job usually doesn't reproduce the issue as seen in our CI as well as in our user report (see link above).

1. Clone open source project https://github.com/openiddict/openiddict-core.git
2. Checkout eb35cbefb700b3f219439431aa489f555a27a5de commit
3. Plug in https://github.com/SonarSource/sonar-dotnet/releases/tag/8.24.0.32949 analyzer (or any older, we didn't change the rule in last 3 years)
4. You can deactivate all rules for performance/noise reasons except S1698
5. Keep rebuilding the solution until the problem occurs dotnet build /t:Rebuild /warnaserror:AD0001

Error observed:

16:44>CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists' 
threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. 
[C:\Project\src\OpenIddict.Server.AspNetCore\OpenIddict.Server.AspNetCore.csproj]
 16:44>Exception occurred with following context:
         Compilation: OpenIddict.Server.AspNetCore
         
         System.NullReferenceException: Object reference not set to an instance of an object.
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.AddTupleTypeName(INamedTypeSymbol symbol)
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.AddNameAndTypeArgumentsOrParameters(INamedTypeSymbol symbol)
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.MinimallyQualify(INamedTypeSymbol symbol)
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.VisitNamedTypeWithoutNullability(INamedTypeSymbol symbol)
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.VisitNamedType(INamedTypeSymbol symbol)
            at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.NamedTypeSymbol.Accept(SymbolVisitor visitor)
            at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.Symbol.Microsoft.CodeAnalysis.ISymbol.Accept(SymbolVisitor visitor)
            at Microsoft.CodeAnalysis.CSharp.SymbolDisplay.ToDisplayParts(ISymbol symbol, SemanticModel semanticModelOpt, Int32 positionOpt, SymbolDisplayFormat format, Boolean minimal)
            at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.Symbol.Microsoft.CodeAnalysis.ISymbol.ToDisplayString(SymbolDisplayFormat format)
            at SonarAnalyzer.Helpers.TypeHelper.IsMatch(ITypeSymbol typeSymbol, KnownType type)
            at SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists.GetEqualsOverrides(ITypeSymbol type)
            at SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists.HasEqualsOverride(ITypeSymbol type)
            at SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists.<>c.<Initialize>b__9_1(INamedTypeSymbol t)
            at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
            at System.Linq.Enumerable.SelectManySingleSelectorIterator`2.MoveNext()
            at System.Collections.Generic.HashSet`1.UnionWith(IEnumerable`1 other)
            at System.Collections.Generic.HashSet`1..ctor(IEnumerable`1 collection, IEqualityComparer`1 comparer)
            at System.Collections.Generic.HashSet`1..ctor(IEnumerable`1 collection)
            at SonarAnalyzer.Helpers.EnumerableExtensions.ToHashSet[T](IEnumerable`1 enumerable, IEqualityComparer`1 equalityComparer)
            at SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists.<>c.<Initialize>b__9_0(CompilationStartAnalysisContext compilationStartContext)
            at SonarAnalyzer.Helpers.SonarAnalysisContext.<>c__DisplayClass46_0`1.<RegisterContextAction>b__0(TContext c)
            at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c.<ExecuteCompilationStartActions>b__44_0(ValueTuple`2 data)
            at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)
         -----
         
         Suppress the following diagnostics to disable this analyzer: S1698
 16:44>Done executing task "Csc".

Adding try/catch and logging the failed symbol ToString() and DeclaringSyntaxReferences..SyntaxTree.FilePath produces:
ToString: OpenIddict.Abstractions.OpenIddictExceptions.ConcurrencyException
FilePath: C:\Project\src\OpenIddict.Abstractions\OpenIddictExceptions.cs

ToString: OpenIddict.Server.OpenIddictServerConfiguration
FilePath: C:\Project\src\OpenIddict.Server\OpenIddictServerConfiguration.cs

ToString: SNINativeMethodWrapper.QTypes
FilePath: - no DeclaringSyntaxReferences

Analyzer Rule source:

https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/src/SonarAnalyzer.CSharp/Rules/ReferenceEqualityCheckWhenEqualsExists.cs

In simplified version, rule registers for RegisterCompilationStartAction (Line 62). There it gathers all the ITypeSymbol (including nested) from Compilation.GlobalNamespace. Then it iterates over each ITypeSymbol and each of it's .BaseType and calls .Is(KnownType.System_Object) (Line 163) that invokes .ToDisplayString().

Expected Behavior:

ToDisplayString() should return some string.

Actual Behavior:

ToDisplayString() throws exception.

[pavel-mikula-sonarsource]

The exception happens inside AddTupleTypeName, but that one gets called only when symbol.IsTupleType:

roslyn/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs

Lines 303 to 307 in d124857

	else if (symbol.IsTupleType && !ShouldDisplayAsValueTuple(symbol))
	{
	AddTupleTypeName(symbol);
	return;
	}

But there are no tuples in the reported ITypeSymbol that caused the exception.

[pavel-mikula-sonarsource]

Today, we had the same issue with another project and more syntax based rules:
Project: https://github.com/PowerShell/PowerShellEditorServices.git
Commit: af3cffa
Rules:
S1200 - AvoidExcessiveClassCoupling
S1643 - StringConcatenationInLoop
S1698 - ReferenceEqualityCheckWhenEqualsExists - known from yesterday
S4056 - SpecifyIFormatProviderOrCultureInfo
S4058 - SpecifyStringComparison
S4834 - ControllingPermissions
S3994, S3995, S3996, S3997, S4005 - UseUriInsteadOfString

[credfeto]

Also seen in 5.0.301 on both windows dotnet command line builds and linux.

[credfeto]

@jaredpar is there any ETA for a fix on this? - at the moment its causing about 50% of builds to fail for us on .net 5.0.301 - we might have to try downgrading to an earlier version of .net but then we won't have any of the security fixes

[jcouv]

@credfeto Thanks for the ping. Let me take a look and get back to you tomorrow.

[jcouv]

@credfeto @pavel-mikula-sonarsource I'm not sure what you mean in the repro steps by activating/deactivating rules. Might you have a commit/branch that would be already configured to trigger the repro? Or alternatively a zip file?

[credfeto]

@jcouv Repo:

https://github.com/funfair-tech/CoinBot

I get this on here just building... note its not all the time on this repo with the same source so best to reproduce using something like

@echo off

:start
dotnet clean
dotnet build --configuration=Release
if ERRORLEVEL 1 goto finish

goto start

:finish

Sample Error logs (Extracted from TC)

CoinBot.Core.Tests -> /mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Core.Tests/bin/Release/net5.0/CoinBot.Core.Tests.dll
[02:21:02]	[Step 4/6]   CoinBot.Core -> /mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Core/bin/Release/net5.0/CoinBot.Core.dll
[02:21:04]	[Step 4/6] CSC : error AD0001: Analyzer 'FunFair.CodeAnalysis.ParameterNameDiagnosticsAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [/mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Clients/CoinBot.Clients.csproj]
[02:21:04]	[Step 4/6] CSC : error AD0001: Analyzer 'FunFair.CodeAnalysis.ParameterOrderingDiagnosticsAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [/mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Clients/CoinBot.Clients.csproj]
[02:21:04]	[Step 4/6] CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.SymbolicExecution.SymbolicExecutionRunner' threw an exception of type 'SonarAnalyzer.SymbolicExecution.SymbolicExecutionException' with message 'Error processing method: GetCurrentPriceCommonAsync ## Method file: /mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Clients/FunFair/FunFairClientBase.cs ## Method line: 80,8 ## Inner exception: System.NullReferenceException: Object reference not set to an instance of an object. ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.AddTupleTypeName(INamedTypeSymbol symbol) ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.AddNameAndTypeArgumentsOrParameters(INamedTypeSymbol symbol) ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.MinimallyQualify(INamedTypeSymbol symbol) ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.VisitNamedTypeWithoutNullability(INamedTypeSymbol symbol) ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplayVisitor.VisitNamedType(INamedTypeSymbol symbol) ##    at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.NamedTypeSymbol.Accept(SymbolVisitor visitor) ##    at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.Symbol.Microsoft.CodeAnalysis.ISymbol.Accept(SymbolVisitor visitor) ##    at Microsoft.CodeAnalysis.CSharp.SymbolDisplay.ToDisplayParts(ISymbol symbol, SemanticModel semanticModelOpt, Int32 positionOpt, SymbolDisplayFormat format, Boolean minimal) ##    at Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel.Symbol.Microsoft.CodeAnalysis.ISymbol.ToDisplayString(SymbolDisplayFormat format) ##    at SonarAnalyzer.Helpers.TypeHelper.IsMatch(ITypeSymbol typeSymbol, KnownType type) ##    at SonarAnalyzer.Helpers.TypeHelper.Is(ITypeSymbol typeSymbol, KnownType type) ##    at SonarAnalyzer.SymbolicExecution.SymbolicValue.Create(ITypeSymbol type) ##    at SonarAnalyzer.SymbolicExecution.AbstractExplodedGraph.EnqueueStartNode() ##    at SonarAnalyzer.SymbolicExecution.AbstractExplodedGraph.Walk() ##    at SonarAnalyzer.Rules.SymbolicExecution.SymbolicExecutionRunner.Analyze(CSharpExplodedGraph explodedGraph, SyntaxNodeAnalysisContext context) ##    at SonarAnalyzer.Extensions.SonarAnalysisContextExtensions.Analyze(CSharpSyntaxNode declarationBody, ISymbol symbol, Action`2 runAnalysis, SyntaxNodeAnalysisContext context) ## '. [/mnt/ssd/buildagent1/work/7c03326b81b9da65/src/CoinBot.Clients/CoinBot.Clients.csproj]

[pavel-mikula-sonarsource]

Hi @jcouv,

Here's ZIP reproducer of the openid-dict project:
https://drive.google.com/file/d/12GAgzv2KCn0RNrnKe7Om_wF0lZRkT-YF/view?usp=sharing

Just run openiddict-core\Build.cmd until it fails, as the problem is nondeterministic. It took 6 tries now on my machine.

CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [c:\_Temp\Roslyn_Repro_53943\openiddict-core\src\OpenIddict.Server.DataProtection\OpenIddict.Server.DataProtection.csproj]
CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.ReferenceEqualityCheckWhenEqualsExists' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [c:\_Temp\Roslyn_Repro_53943\openiddict-core\src\OpenIddict.Server.AspNetCore\OpenIddict.Server.AspNetCore.csproj]
    118 Warning(s)
    2 Error(s)

[pavel-mikula-sonarsource]

The ZIP link will probably stop working next week. Here's the same content:
https://github.com/pavel-mikula-sonarsource/Roslyn_Repro_53943

[jcouv]

Thanks for the additional info. I wasn't able to download @pavel-mikula-sonarsource's zip file yet (requested permission).
But I was able to repro using @credfeto's self-contained repro.

[pavel-mikula-sonarsource]

Hi @jcouv , I just approved the sharing. Same content is in the repo link from my previous message.

[Corniel]

I can confirm this issue.

[jcouv]

I'm still stuck on repro'ing to investigate this.

I'd be able to repro with @credfeto's steps (CoinBot) but a few days later it stopped repro'ing. Not sure why.
I was not able to complete the repro steps from @pavel-mikula-sonarsource. But there is no openiddict-core.editorconfig file in this repo and I'm not sure what "enable only one rule" means.

Does anyone have a crash dump file?

[credfeto]

@jcouv I've had it happen several times today in various projects. Not yet made it fail in the coinbot project; but have just created a branch with all changes rolled back to as it was when I referenced it as an example in June. Is there an easy way to get a crashdump without having to attach to build processes every time I do a build?

Its definitely happening less than it was back in June for me.

[jcouv]

@credfeto If you're getting crashes and you'd like those crashes to produce crash dumps, you can set a registry key that specifies the dump output folder and the number of dump files to keep. Here's some more information.

[pavel-mikula-sonarsource]

I was not able to complete the repro steps from @pavel-mikula-sonarsource. But there is no openiddict-core.editorconfig file in this repo and I'm not sure what "enable only one rule" means.

openiddict-core.editorconfig should have been openiddict-core\.editorconfig, there was a missing escape in the .md file :/ Readme.md is fixed now.

You can reproduce it with all rules enabled, but the build will be slower. Enabling one rule means just this:
https://github.com/pavel-mikula-sonarsource/Roslyn_Repro_53943/blob/152fed3f9c285d51d7a8daeb4ab455f2a8522404/openiddict-core/.editorconfig#L194

[pavel-mikula-sonarsource]

I just reproduced it with these steps:
Clone https://github.com/pavel-mikula-sonarsource/Roslyn_Repro_53943
Run build 11 times openiddict-core/Build.cmd

The first 10 runs were successful, as the issue is nondeterministic.

I did set up the dump via registry, but this didn't produce a dump file because it is a caught exception. It's not a crash.

From the logs:

Using "Csc" task from assembly "C:\Program Files\dotnet\sdk\5.0.400\Roslyn\Microsoft.Build.Tasks.CodeAnalysis.dll"

That file has version 3.11.4.37306

[pavel-mikula-sonarsource]

This recently started appearing more often in our CI again

[RikkiGibson]

It doesn't appear that this bug was fixed. Moving to 17.9 as I don't think we have time to do it in 17.8.

[pavel-mikula-sonarsource]

Thanks Rikki, I confirm we've seen few of these this week.

[jaredpar]

Does anyone have a recent repro of this issue? I tried building several of the examples in the issue in a loop and none exhibitted the described crash behavior. Happy to take non-deterministic repos and just loop the build.

[jjonescz]

https://github.com/jjonescz/RoslynIssue53943 - run repro.ps1 from openiddict-core folder.

[jaredpar]

@jjonescz ah okay I can repro with that setup. Thanks! I was roughly trying the same approach but wasn't getting the crash (ran 50+ iterations but yours crashes after ~3). Only difference I can see is the use of the 5.0.400 SDK. Now that it repros though I'm hoping I can get it to do so with the debugger attached.

This investigation though makes me wonder if we need a better debug option here for the compiler. Essentially a feature flag such that we terminate the compiler process using FailFast when there is an unhandled anlayzer exception. That would be very useful in scenarios like this at getting dumps from customer machines to look at.

[jaredpar]

@jjonescz it does appear that the important difference in our setups is the use of the 5.0.400 SDK. I can reliably get the error to happen there but when I move to any modern SDK the error stops. This morning I effectively ran your setup with a Debug compiler from main and could not reproduce the issue here. Let it run for ~50 iterations. It seems plausible then that the issue was fixed between then and now.

Are there any repros using more recent SDKs that I could look at?

[jjonescz]

@jaredpar Yes, I can also repro only with 5.0.400 SDK. There have been more recent reports (like #69945 or #53943 (comment)) but nothing I could repro locally.

@pavel-mikula-sonarsource You've said you hit this regularly in CI. What SDK version are you using there? Is the CI run public or can it be reproduced locally?

[pavel-mikula-sonarsource]

Hi @jjonescz,
As a reminder, we write analyzers. And we analyze public projects as part of our integration testing.

The last occurrence I was able to find was on the 12th of October 2023, while analyzing https://github.com/EventStore/EventStore.git commit 494147e. This project uses global.json with { "sdk": { "version": "5.0.0", "rollForward": "feature" } }

The other occurrence I was able to find was on the same day while analyzing https://github.com/shadowsocks/shadowsocks-windows.git commit 8fafef7c5751809751d8eef361e3c39caef75261. There is no global.json in the repo and we don't craft a custom one. The project itself has a target framework of .NET 5. I don't see the exact SDK version used in the logs, as it was built with

C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\Bin\MSBuild.exe /bl:C:/Windows/SystemTemp/cirrus-ci-build\build.binlog /clp:Summary;ShowTimestamp;ShowEventId /maxcpucount:8 /nr:false /p:DeployExtension=false /p:reportanalyzer=true /t:Rebuild /v:m /warnaserror:AD0001;CS8032;BC42376 shadowsocks-windows.sln

Would .NET 5 SDK be used here too?

Older CI logs were already pruned, so I can't find previous occurrences, unfortunately.

We run the CI integration tests daily and it appears very randomly.

[jaredpar]

Would .NET 5 SDK be used here too?

It's hard to say what version is used without looking at a binary log or even some msbuild diagnostic data. Given that build is initiated from VS 2019 though it's not going to be anything newer than .NET 5 SDK though.

[jaredpar]

Given there are no repros on NET 6 or later, going to assume that one of our other race condition fixes in tuples took care of this issue as well. If someone can repro this on NET 6, or even better NET 7, please let us know and we'll dig into it.