Assert in TaintedDataAnalysis.TaintedDataSymbolMap<>.GetInfosForType

[mavasani]

1. Attempt to build the roslyn-analyzers repo with the latest analyzer debug bits built from master

Got:
Assert fires:

roslyn-analyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataSymbolMap.cs

Line 86 in c844bfb

Debug.Assert(namedTypeSymbol != null);

>	Microsoft.NetCore.Analyzers.dll!Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis.TaintedDataSymbolMap<Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis.SourceInfo>.GetInfosForType(Microsoft.CodeAnalysis.INamedTypeSymbol namedTypeSymbol) Line 86	C#
 	Microsoft.NetCore.Analyzers.dll!Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis.TaintedDataSymbolMapExtensions.IsSourceConstantArrayOfType(Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis.TaintedDataSymbolMap<Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis.SourceInfo> sourceSymbolMap, Microsoft.CodeAnalysis.IArrayTypeSymbol arrayTypeSymbol) Line 85	C#
 	Microsoft.NetCore.Analyzers.dll!Microsoft.NetCore.Analyzers.Security.SourceTriggeredTaintedDataAnalyzerBase.Initialize.AnonymousMethod__4(Microsoft.CodeAnalysis.Diagnostics.OperationAnalysisContext operationAnalysisContext) Line 190	C#

Assert seems benign as we do handle the null value in the very next statement.

[dotpaul]

@LLLXXXCCC can you look at this one as well? Also seems related to the TaintedDataAnalysis changes for constant arrays.

[mavasani]

@LLLXXXCCC This should be fixed now?

[LLLXXXCCC]

It is supposed to be caused by the same reason with #2707 . This problem was solved. But I'll extend it to handle hard code in jagged array. @dotpaul

[dotpaul]

I doubt this issue has same root cause as #2707, because I don't remember the assert firing when running #2707's test case. I vaguely remember the assert coming from a different place.

@LLLXXXCCC were you able to repro? It'd be good to understand exactly what code made the assert fired. Maybe the fix will end up being the same, or maybe we'll find there's another code pattern we need to handle.

In any case, we should have at least one unit test containing code that would have fired the assert.

[mavasani]

@dotpaul @LLLXXXCCC This should now be fixed, correct?

[dotpaul]

@dotpaul @LLLXXXCCC This should now be fixed, correct?

I believe so.