Performance fix for AnalysisEntityMapAbstractDomain.Merge algorithm

src/Microsoft.CodeAnalysis.FlowAnalysis.Utilities/PublicAPI.Unshipped.txt

@@ -124,7 +124,7 @@ Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityFactory.TryCreateForT
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityFactory.TryGetForFlowCapture(Microsoft.CodeAnalysis.FlowAnalysis.CaptureId captureId, out Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity analysisEntity) -> bool
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityFactory.TryGetForInterproceduralAnalysis(Microsoft.CodeAnalysis.IOperation operation, out Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity analysisEntity) -> bool
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityMapAbstractDomain<TValue>
-Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityMapAbstractDomain<TValue>.AnalysisEntityMapAbstractDomain(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractValueDomain<TValue> valueDomain) -> void
+Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityMapAbstractDomain<TValue>.AnalysisEntityMapAbstractDomain(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractValueDomain<TValue> valueDomain, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAnalysisResult pointsToAnalysisResultOpt) -> void
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.ArgumentInfo<TAbstractAnalysisValue>
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.ArgumentInfo<TAbstractAnalysisValue>.AnalysisEntityOpt.get -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity
 Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.ArgumentInfo<TAbstractAnalysisValue>.ArgumentInfo(Microsoft.CodeAnalysis.IOperation operation, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntity analysisEntityOpt, Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis.PointsToAbstractValue instanceLocation, TAbstractAnalysisValue value) -> void

src/Microsoft.NetCore.Analyzers/UnitTests/Security/ReviewCodeForSqlInjectionVulnerabilitiesTests.cs

@@ -3066,5 +3066,197 @@ protected virtual bool HasUrl(IContext filterContext)
 }
 ");
         }
+
+        [Fact]
+        public void LotsOfAnalysisEntities_1()
+        {
+            VerifyCSharpWithDependencies(@"
+using System;
+using System.Data;
+using System.Data.SqlClient;
+using System.Web;
+
+namespace TestNamespace
+{
+    public class DataStructure
+    {
+        public string StringProperty { get; set; }
+    }
+
+    public class ExampleClass
+    {
+        public SqlCommand Something(HttpRequest request)
+        {
+            string name = request.Form[""product_name""];
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            if ((new Random()).Next(6) == 4) 
+            {
+                return null;
+            }
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            for (int i = 0; i < 3; i++)
+            {
+                new DataStructure()
+                {
+                    StringProperty = ""This is tainted: "" + name,
+                };
+            }
+
+            return null;
+        }
+    }
+}");
+        }
+
+        [Fact]
+        public void LotsOfAnalysisEntities_2()
+        {
+            VerifyCSharpWithDependencies(@"
+using System;
+using System.Data;
+using System.Data.SqlClient;
+using System.Web;
+
+namespace TestNamespace
+{
+    public class DataStructure
+    {
+        public string StringProperty { get; set; }
+    }
+
+    public class ExampleClass
+    {
+        public SqlCommand Something(HttpRequest request)
+        {
+            string name = request.Form[""product_name""];
+
+            var d1 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d2 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d3 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d4 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d5 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d6 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            if ((new Random()).Next(6) == 4) 
+            {
+                return null;
+            }
+
+            var d7 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d8 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d9 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d10 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            var d11 = new DataStructure()
+            {
+                StringProperty = ""This is tainted: "" + name,
+            };
+
+            for (int i = 0; i < 3; i++)
+            {
+                var d12 = new DataStructure()
+                {
+                    StringProperty = ""This is tainted: "" + name,
+                };
+            }
+
+            return null;
+        }
+    }
+}");
+        }
     }
 }

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/DefaultPointsToValueGenerator.cs

@@ -39,7 +39,7 @@ public PointsToAbstractValue GetOrCreateDefaultValue(AnalysisEntity analysisEnti
                 }
 
                 value = PointsToAbstractValue.Create(AbstractLocation.CreateAnalysisEntityDefaultLocation(analysisEntity), mayBeNull: true);
-                _trackedEntitiesBuilder.AllEntities.Add(analysisEntity);
+                _trackedEntitiesBuilder.AddEntity(analysisEntity);
                 _defaultPointsToValueMapBuilder.Add(analysisEntity, value);
             }
 

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.CorePointsToAnalysisDataDomain.cs

@@ -18,7 +18,7 @@ public partial class PointsToAnalysis : ForwardDataFlowAnalysis<PointsToAnalysis
         private sealed class CorePointsToAnalysisDataDomain : AnalysisEntityMapAbstractDomain<PointsToAbstractValue>
         {
             public CorePointsToAnalysisDataDomain(DefaultPointsToValueGenerator defaultPointsToValueGenerator, AbstractValueDomain<PointsToAbstractValue> valueDomain)
-                : base(valueDomain)
+                : base(valueDomain, defaultPointsToValueGenerator.IsTrackedEntity)
             {
                 DefaultPointsToValueGenerator = defaultPointsToValueGenerator;
             }
@@ -29,8 +29,7 @@ public CorePointsToAnalysisDataDomain(DefaultPointsToValueGenerator defaultPoint
 
             protected override bool CanSkipNewEntry(AnalysisEntity analysisEntity, PointsToAbstractValue value)
                 => value.Kind == PointsToAbstractValueKind.Unknown ||
-                    !DefaultPointsToValueGenerator.IsTrackedEntity(analysisEntity) ||
-                    value == GetDefaultValue(analysisEntity);
+                   value == GetDefaultValue(analysisEntity);
 
             protected override void AssertValidEntryForMergedMap(AnalysisEntity analysisEntity, PointsToAbstractValue value)
             {

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToDataFlowOperationVisitor.cs

@@ -20,7 +20,6 @@ public partial class PointsToAnalysis : ForwardDataFlowAnalysis<PointsToAnalysis
         private sealed class PointsToDataFlowOperationVisitor :
             AnalysisEntityDataFlowOperationVisitor<PointsToAnalysisData, PointsToAnalysisContext, PointsToAnalysisResult, PointsToAbstractValue>
         {
-            private readonly TrackedEntitiesBuilder _trackedEntitiesBuilder;
             private readonly DefaultPointsToValueGenerator _defaultPointsToValueGenerator;
             private readonly PointsToAnalysisDomain _pointsToAnalysisDomain;
             private readonly PooledDictionary<IOperation, ImmutableHashSet<AbstractLocation>.Builder> _escapedOperationLocationsBuilder;
@@ -34,7 +33,7 @@ private sealed class PointsToDataFlowOperationVisitor :
                 PointsToAnalysisContext analysisContext)
                 : base(analysisContext)
             {
-                _trackedEntitiesBuilder = trackedEntitiesBuilder;
+                TrackedEntitiesBuilder = trackedEntitiesBuilder;
                 _defaultPointsToValueGenerator = defaultPointsToValueGenerator;
                 _pointsToAnalysisDomain = pointsToAnalysisDomain;
                 _escapedOperationLocationsBuilder = PooledDictionary<IOperation, ImmutableHashSet<AbstractLocation>.Builder>.GetInstance();
@@ -44,6 +43,8 @@ private sealed class PointsToDataFlowOperationVisitor :
                 analysisContext.InterproceduralAnalysisDataOpt?.InitialAnalysisData.AssertValidPointsToAnalysisData();
             }
 
+            internal TrackedEntitiesBuilder TrackedEntitiesBuilder { get; }
+
             public ImmutableDictionary<IOperation, ImmutableHashSet<AbstractLocation>> GetEscapedLocationsThroughOperationsMap()
                 => GetEscapedAbstractLocationsMapAndFreeBuilder(_escapedOperationLocationsBuilder);
 
@@ -119,7 +120,7 @@ protected override void AddTrackedEntities(PointsToAnalysisData analysisData, Ha
                     return;
                 }
 
-                foreach (var entity in _trackedEntitiesBuilder.AllEntities)
+                foreach (var entity in TrackedEntitiesBuilder.EnumerateEntities())
                 {
                     if (analysisData.HasAbstractValue(entity) ||
                         !forInterproceduralAnalysis && _defaultPointsToValueGenerator.IsTrackedEntity(entity))
@@ -174,7 +175,7 @@ protected override void SetAbstractValue(AnalysisEntity analysisEntity, PointsTo
                     }
 
                     SetAbstractValueCore(CurrentAnalysisData, analysisEntity, value);
-                    _trackedEntitiesBuilder.AllEntities.Add(analysisEntity);
+                    TrackedEntitiesBuilder.AddEntity(analysisEntity);
                 }
             }
 

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.Collections.Immutable;
 using System.Diagnostics;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
@@ -97,7 +98,8 @@ protected override PointsToAnalysisResult ToResult(PointsToAnalysisContext analy
                 dataFlowAnalysisResult,
                 operationVisitor.GetEscapedLocationsThroughOperationsMap(),
                 operationVisitor.GetEscapedLocationsThroughReturnValuesMap(),
-                operationVisitor.GetEscapedLocationsThroughEntitiesMap());
+                operationVisitor.GetEscapedLocationsThroughEntitiesMap(),
+                operationVisitor.TrackedEntitiesBuilder.ToImmutable());
         }
         protected override PointsToBlockAnalysisResult ToBlockResult(BasicBlock basicBlock, PointsToAnalysisData blockAnalysisData)
             => new PointsToBlockAnalysisResult(basicBlock, blockAnalysisData);

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysisResult.cs

@@ -12,17 +12,20 @@ public sealed class PointsToAnalysisResult : DataFlowAnalysisResult<PointsToBloc
         private readonly ImmutableDictionary<IOperation, ImmutableHashSet<AbstractLocation>> _escapedLocationsThroughOperationsMap;
         private readonly ImmutableDictionary<IOperation, ImmutableHashSet<AbstractLocation>> _escapedLocationsThroughReturnValuesMap;
         private readonly ImmutableDictionary<AnalysisEntity, ImmutableHashSet<AbstractLocation>> _escapedLocationsThroughEntitiesMap;
+        private readonly ImmutableHashSet<AnalysisEntity> _trackedEntities;
 
         internal PointsToAnalysisResult(
             DataFlowAnalysisResult<PointsToBlockAnalysisResult, PointsToAbstractValue> corePointsToAnalysisResult,
             ImmutableDictionary<IOperation, ImmutableHashSet<AbstractLocation>> escapedLocationsThroughOperationsMap,
             ImmutableDictionary<IOperation, ImmutableHashSet<AbstractLocation>> escapedLocationsThroughReturnValuesMap,
-            ImmutableDictionary<AnalysisEntity, ImmutableHashSet<AbstractLocation>> escapedLocationsThroughEntitiesMap)
+            ImmutableDictionary<AnalysisEntity, ImmutableHashSet<AbstractLocation>> escapedLocationsThroughEntitiesMap,
+            ImmutableHashSet<AnalysisEntity> trackedEntities)
             : base(corePointsToAnalysisResult)
         {
             _escapedLocationsThroughOperationsMap = escapedLocationsThroughOperationsMap;
             _escapedLocationsThroughReturnValuesMap = escapedLocationsThroughReturnValuesMap;
             _escapedLocationsThroughEntitiesMap = escapedLocationsThroughEntitiesMap;
+            _trackedEntities = trackedEntities;
         }
 
         public ImmutableHashSet<AbstractLocation> GetEscapedAbstractLocations(IOperation operation)
@@ -44,5 +47,8 @@ public ImmutableHashSet<AbstractLocation> GetEscapedAbstractLocations(AnalysisEn
 
             return ImmutableHashSet<AbstractLocation>.Empty;
         }
+
+        internal bool IsTrackedEntity(AnalysisEntity analysisEntity)
+            => _trackedEntities.Contains(analysisEntity);
     }
 }

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/TrackedEntitiesBuilder.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
 using Analyzer.Utilities.PooledObjects;
 
 namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis
@@ -10,16 +12,29 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis
     /// </summary>
     internal sealed class TrackedEntitiesBuilder : IDisposable
     {
+        /// <summary>
+        /// Stores all the tracked entities.
+        /// NOTE: Entities added to this set should not be removed.
+        /// </summary>
+        private PooledHashSet<AnalysisEntity> _allEntities { get; }
+
         public TrackedEntitiesBuilder()
         {
-            AllEntities = PooledHashSet<AnalysisEntity>.GetInstance();
+            _allEntities = PooledHashSet<AnalysisEntity>.GetInstance();
         }
 
-        public PooledHashSet<AnalysisEntity> AllEntities { get; }
-
         public void Dispose()
         {
-            AllEntities.Free();
+            _allEntities.Free();
         }
+
+        public void AddEntity(AnalysisEntity analysisEntity)
+            => _allEntities.Add(analysisEntity);
+
+        public IEnumerable<AnalysisEntity> EnumerateEntities()
+            => _allEntities;
+
+        public ImmutableHashSet<AnalysisEntity> ToImmutable()
+            => _allEntities.ToImmutableHashSet();
     }
 }

src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.CoreTaintedDataAnalysisDataDomain.cs

@@ -1,16 +1,16 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow;
+using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis;
 
 namespace Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis
 {
     internal partial class TaintedDataAnalysis
     {
         private sealed class CoreTaintedDataAnalysisDataDomain : AnalysisEntityMapAbstractDomain<TaintedDataAbstractValue>
         {
-            public static readonly CoreTaintedDataAnalysisDataDomain Instance = new CoreTaintedDataAnalysisDataDomain(TaintedDataAbstractValueDomain.Default);
-
-            private CoreTaintedDataAnalysisDataDomain(TaintedDataAbstractValueDomain valueDomain) : base(valueDomain)
+            public CoreTaintedDataAnalysisDataDomain(PointsToAnalysisResult pointsToAnalysisResultOpt)
+                : base(TaintedDataAbstractValueDomain.Default, pointsToAnalysisResultOpt)
             {
             }