codeql icon indicating copy to clipboard operation
codeql copied to clipboard

C#: Negative summaries (ie. no flow through)

Open michaelnebel opened this issue 3 years ago • 8 comments
trafficstars

In this PR we introduce the concept of negative flow summaries, which is a summary of a callable stating that there is no flow via this callable. The implementation introduces a new class NegativeSummaryModelCsv class, which re-uses parts of the syntax for summaries. The CSV row syntax is namespace;type;name;signature;provenance.

  • The implementation is made for C# and Java (and dummy/empty implementation has been made for Swift and Ruby)
  • The Framework code can parse the negative summaries and new predicates have been introduced to extract the negative summaries from NegativeSummaryModelCsv subclasses.
  • Validation of the negative summaries is included in the CsvValidation.
  • The model generator(s) have been improved to also output a negative summary in case no flow through summary exists.
    • Furthermore it turned out that we need to discard Finalizers (destructors) in the model generation.
  • Telemetry queries now respect the negative summaries. That is, if a callable has a negative flow summary it will not be reported as missing by the telemetry queries.
  • Negative summaries are not included and not counted towards flow summaries in general (in the model coverage queries).

Open questions

  • Should we extends the model coverage functionality to include these summaries or alternatively make another category of coverage where this is reported?
  • Should we make some DCA support for this as well?

@hvitved Any insights about the open questions and/or the implementation itself will be valuable before continuing the work.

michaelnebel avatar Jul 20 '22 13:07 michaelnebel

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,51183,28,5
-    Others,"``Dapper``, ``JsonToItemsTaskFactory``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``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.NETCore.Platforms.BuildTasks``, ``Microsoft.VisualBasic``, ``Microsoft.Win32``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,554,137,
+    Others,"``AssemblyStripper``, ``Dapper``, ``Generators``, ``Internal``, ``JsonToItemsTaskFactory``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``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.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Internal``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NETCore.Platforms.BuildTasks``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Microsoft.Workload.Build.Tasks``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,3330,137,
-    Totals,,3,12599,359,5
+    Totals,,3,54520,359,5
  • Changes to framework-coverage-csharp.csv:
- package,sink,source,summary,sink:code,sink:html,sink:remote,sink:sql,sink:xss,source:local,summary:taint,summary:value
+ package,sink,source,summary,sink:code,sink:html,sink:remote,sink:sql,sink:xss,source:local,summary:none,summary:taint,summary:value
+ AssemblyStripper,,,1,,,,,,,1,,
- Dapper,55,,,,,,55,,,,
+ Dapper,55,,,,,,55,,,,,
+ Generators,,,2,,,,,,,2,,
+ Internal,,,32,,,,,,,32,,
- JsonToItemsTaskFactory,,,7,,,,,,,7,
+ JsonToItemsTaskFactory,,,33,,,,,,,26,7,
- Microsoft.ApplicationBlocks.Data,28,,,,,,28,,,,
+ Microsoft.ApplicationBlocks.Data,28,,,,,,28,,,,,
- Microsoft.CSharp,,,24,,,,,,,24,
+ Microsoft.CSharp,,,36,,,,,,,12,24,
+ Microsoft.DotNet.Build.Tasks,,,47,,,,,,,47,,
+ Microsoft.DotNet.PlatformAbstractions,,,6,,,,,,,6,,
- Microsoft.EntityFrameworkCore,6,,,,,,6,,,,
+ Microsoft.EntityFrameworkCore,6,,,,,,6,,,,,
- Microsoft.Extensions.Caching.Distributed,,,15,,,,,,,15,
+ Microsoft.Extensions.Caching.Distributed,,,41,,,,,,,26,15,
- Microsoft.Extensions.Caching.Memory,,,46,,,,,,,45,1
+ Microsoft.Extensions.Caching.Memory,,,92,,,,,,,46,45,1
- Microsoft.Extensions.Configuration,,,83,,,,,,,80,3
+ Microsoft.Extensions.Configuration,,,236,,,,,,,153,80,3
- Microsoft.Extensions.DependencyInjection,,,62,,,,,,,62,
+ Microsoft.Extensions.DependencyInjection,,,283,,,,,,,221,62,
- Microsoft.Extensions.DependencyModel,,,12,,,,,,,12,
+ Microsoft.Extensions.DependencyModel,,,115,,,,,,,103,12,
- Microsoft.Extensions.FileProviders,,,15,,,,,,,15,
+ Microsoft.Extensions.FileProviders,,,85,,,,,,,70,15,
- Microsoft.Extensions.FileSystemGlobbing,,,15,,,,,,,13,2
+ Microsoft.Extensions.FileSystemGlobbing,,,122,,,,,,,107,13,2
- Microsoft.Extensions.Hosting,,,17,,,,,,,16,1
+ Microsoft.Extensions.Hosting,,,123,,,,,,,106,16,1
- Microsoft.Extensions.Http,,,10,,,,,,,10,
+ Microsoft.Extensions.Http,,,25,,,,,,,15,10,
+ Microsoft.Extensions.Internal,,,2,,,,,,,2,,
- Microsoft.Extensions.Logging,,,37,,,,,,,37,
+ Microsoft.Extensions.Logging,,,245,,,,,,,208,37,
- Microsoft.Extensions.Options,,,8,,,,,,,8,
+ Microsoft.Extensions.Options,,,181,,,,,,,173,8,
- Microsoft.Extensions.Primitives,,,63,,,,,,,63,
+ Microsoft.Extensions.Primitives,,,142,,,,,,,79,63,
- Microsoft.Interop,,,27,,,,,,,27,
+ Microsoft.Interop,,,417,,,,,,,390,27,
- Microsoft.NET.Build.Tasks,,,1,,,,,,,1,
+ Microsoft.NET.Build.Tasks,,,82,,,,,,,81,1,
- Microsoft.NETCore.Platforms.BuildTasks,,,4,,,,,,,4,
+ Microsoft.NETCore.Platforms.BuildTasks,,,84,,,,,,,80,4,
- Microsoft.VisualBasic,,,9,,,,,,,5,4
+ Microsoft.VisualBasic,,,646,,,,,,,637,5,4
+ Microsoft.WebAssembly.Build.Tasks,,,26,,,,,,,26,,
- Microsoft.Win32,,,8,,,,,,,8,
+ Microsoft.Win32,,,122,,,,,,,114,8,
+ Microsoft.Workload.Build.Tasks,,,13,,,,,,,13,,
- MySql.Data.MySqlClient,48,,,,,,48,,,,
+ MySql.Data.MySqlClient,48,,,,,,48,,,,,
- Newtonsoft.Json,,,91,,,,,,,73,18
+ Newtonsoft.Json,,,91,,,,,,,,73,18
- ServiceStack,194,,7,27,,75,92,,,7,
+ ServiceStack,194,,7,27,,75,92,,,,7,
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,51183,,4,,23,1,3,39387,9854,1942

github-actions[bot] avatar Jul 20 '22 13:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 21 '22 09:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 22 '22 12:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 26 '22 14:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 27 '22 10:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 27 '22 13:07 github-actions[bot]

:warning: 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``",3,12038,28,5
+    System,"``System.*``, ``System``",3,11796,28,5
-    Totals,,3,12599,359,5
+    Totals,,3,12357,359,5
  • Changes to framework-coverage-csharp.csv:
- System,28,3,12038,,4,,23,1,3,10096,1942
+ System,28,3,11796,,4,,23,1,3,9854,1942

github-actions[bot] avatar Jul 28 '22 11:07 github-actions[bot]

DCA tests have executed. (1) Performance of nightly queries show no performance regressions or change in alerts. (2) Performance of capture model show no performance regressions, but a significant change in the number of output rows. For .NET Runtime there is an increase in around 40k rows (which is probably acceptable as we have 10k positive rows). For .NET EFCore we don't know how many positive rows we have but there is a total of around 70k rows. Will try and run the Capture model queries again on all DCA projects such that we get an understanding of the magnitude of rows.

michaelnebel avatar Jul 29 '22 06:07 michaelnebel

(1) DCA for nightly looks fine. No problem with query execution. (2) DCA run for the Models as Data queries themselves hit an OOM exception. This should not block merging this PR, but I will investigate. @aschackmull : Since you last review'ed there has been a rebase, but no other changes.

michaelnebel avatar Aug 24 '22 07:08 michaelnebel

(1) DCA for nightly looks fine. No problem with query execution. (2) DCA run for the Models as Data queries themselves hit an OOM exception. This should not block merging this PR, but I will investigate. @aschackmull : Since you last review'ed there has been a rebase, but no other changes.

@aschackmull : Had to rebase yet again. No other changes. Please "review" again. I will keep an eye on the CI and merge ASAP if it becomes possible.

michaelnebel avatar Aug 24 '22 08:08 michaelnebel