C#: Add generated higher order models for .NET8 Runtime.

[michaelnebel]

I spot checked some of the added LINQ models. They look good. Some models related to key comparing is missing, but I don't think this is due to the higher order model generation (I think this is related to flow for dictionaries).

DCA looks good, but unfortunately, we don't find any new results.

[github-actions]

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

• Changes to framework-coverage-csharp.rst:

-    System,"``System.*``, ``System``",47,10313,54,5
+    System,"``System.*``, ``System``",47,10818,54,5
-    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",57,1848,148,
+    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",57,2068,148,
-    Totals,,104,12168,396,5
+    Totals,,104,12893,396,5

• Changes to framework-coverage-csharp.csv:

- Internal.TypeSystem,,,328,,,,,,,,,,,,,,,,,,,201,127
+ Internal.TypeSystem,,,329,,,,,,,,,,,,,,,,,,,201,128
- Microsoft.Diagnostics.Tools.Pgo,,,23,,,,,,,,,,,,,,,,,,,2,21
+ Microsoft.Diagnostics.Tools.Pgo,,,25,,,,,,,,,,,,,,,,,,,2,23
+ Microsoft.DotNet.PlatformAbstractions,,,1,,,,,,,,,,,,,,,,,,,1,
- Microsoft.Extensions.Caching.Memory,,,31,,,,,,,,,,,,,,,,,,,5,26
+ Microsoft.Extensions.Caching.Memory,,,37,,,,,,,,,,,,,,,,,,,5,32
- Microsoft.Extensions.Configuration,,3,91,,,,,,,,,,,,,3,,,,,,25,66
+ Microsoft.Extensions.Configuration,,3,101,,,,,,,,,,,,,3,,,,,,29,72
- Microsoft.Extensions.DependencyInjection,,,130,,,,,,,,,,,,,,,,,,,17,113
+ Microsoft.Extensions.DependencyInjection,,,202,,,,,,,,,,,,,,,,,,,15,187
- Microsoft.Extensions.FileSystemGlobbing,,,22,,,,,,,,,,,,,,,,,,,11,11
+ Microsoft.Extensions.FileSystemGlobbing,,,21,,,,,,,,,,,,,,,,,,,10,11
- Microsoft.Extensions.Hosting,,,39,,,,,,,,,,,,,,,,,,,29,10
+ Microsoft.Extensions.Hosting,,,58,,,,,,,,,,,,,,,,,,,29,29
- Microsoft.Extensions.Logging,,,64,,,,,,,,,,,,,,,,,,,25,39
+ Microsoft.Extensions.Logging,,,91,,,,,,,,,,,,,,,,,,,25,66
- Microsoft.Extensions.Options,,,14,,,,,,,,,,,,,,,,,,,14,
+ Microsoft.Extensions.Options,,,68,,,,,,,,,,,,,,,,,,,44,24
- Microsoft.Extensions.Primitives,,,72,,,,,,,,,,,,,,,,,,,67,5
+ Microsoft.Extensions.Primitives,,,73,,,,,,,,,,,,,,,,,,,67,6
- Microsoft.Interop,,,137,,,,,,,,,,,,,,,,,,,70,67
+ Microsoft.Interop,,,159,,,,,,,,,,,,,,,,,,,75,84
- Mono.Linker,,,287,,,,,,,,,,,,,,,,,,,145,142
+ Mono.Linker,,,293,,,,,,,,,,,,,,,,,,,145,148
- System,54,47,10313,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5351,4962
+ System,54,47,10818,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5511,5307

[hvitved]

Very nice! The few summaries that I checked look great. Will be interesting to see what QA says.

[michaelnebel]

Very nice! The few summaries that I checked look great. Will be interesting to see what QA says.

Do you think it is worth running a QA experiment before merging this branch?

[hvitved]

Do you think it is worth running a QA experiment before merging this branch?

I think it's fine to wait until the next release QA run.

[tamasvajk]

Spot checked a couple of models. I highlighted some that look odd.

[tamasvajk]

Do these summaries make sense? I think generally Equals would not propagate taint from arg to qualifier. Similarly I don't think GetHashCode should propagate flow either. There could be implementations that cache the arguments, and therefore taint is propagated to the qualifier, but that behavior doesn't match the general purpose of these methods.

[michaelnebel]

No, these are not very good summaries. The provenance of the models are df-generated, so these models come from the "old" heuristic for detecting models.
In this case, exactly as you guess, because there exists an implementation, where the heuristic derives that flow propgates to this (as there is a field containing a Func which is applied to the arguments) and then the model is lifted.
One could argue yet another improvement we could consider is only to lift models in case the models are functional - but I think that is out of scope for this PR.

[tamasvajk]

The same comment applies as with System.Collections.IEqualityComparer

[michaelnebel]

I haven't checked this concretely, but with very high probability it is the same issue as mentioned above.

[tamasvajk]

I'd expect an Argument[0] to Argument[this] summary here.

[michaelnebel]

The model generator considers simple types as sanitisers. The backing field containing the hash code is a long, so we shouldn't expect the Argument[0] -> Argument[this] model.
However, the reason we are getting the Argument[0] -> Argument[1] (bad) model is for the same reason that we get the IEqualityComparer<TValue>.Compare models (there exists an implementation that we can dispatch to, which is considered to manipulate the state of this for the IEqualityComparer).

[tamasvajk]

Are the removals in this file expected?

[michaelnebel]

Yes, these removals are of neutrals and we don't generate a neutral since we are generating a model for a base implementation of this method

 - ["System.Reflection", "MethodBase", True, "Invoke", "(System.Object,System.Reflection.BindingFlags,System.Reflection.Binder,System.Object[],System.Globalization.CultureInfo)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"]

[michaelnebel]

@tamasvajk : Thank you for the review - I have tried to answer the questions the best I can. Thank you for pinpointing some good examples for further inspection. I don't think there are any of these cases that requires that we make an immediate improvement before releasing the models in their current state.