Fix 4.1.0 issue: System.InvalidOperationException: `Sequence contai…

…ns more than one matching element` when `--trace` is provided (#896)

* Fix trace issue

* Add Release History

* Fix override

* Fix test

* Update Test

* Update Release History

* Fix hardcode string

README.md

@@ -38,7 +38,7 @@ If you only want to run the Binskim tool without installing anything, then you c
 
 | Argument (short form, long form) | Meaning |
 | -------------------------------- | ------- |
-| **`--trace`** | Execution traces, expressed as a semicolon-delimited list enclosed in double quotes, that should be emitted to the console and log file (if appropriate). Valid values: PdbLoad. |
+| **`--trace`** | Execution traces, expressed as a semicolon-delimited list enclosed in double quotes, that should be emitted to the console and log file (if appropriate). Valid values: PdbLoad, ScanTime, RuleScanTime, PeakWorkingSet, TargetsScanned, ResultsSummary. |
 | **`--sympath`** | Symbol paths, expressed as a semicolon-delimited list enclosed in double quotes. (e.g. `SRV http://msdl.microsoft.com/download/symbols or Cache d:\symbols;Srv http://symweb`) |
 | **`--local-symbol-directories`** | Local directory paths, expressed as a semicolon-delimited list enclosed in double quotes, that will be examined when attempting to locate PDBs. |
 | **`-o, --output`** | File path used to write and output analysis using [SARIF](https://github.com/Microsoft/sarif-sdk) |

ReleaseHistory.md

@@ -16,6 +16,8 @@
 - NEW => new feature 
 
 ## UNRELEASED
+* BUG: Fix `System.InvalidOperationException`: `Sequence contains more than one matching element` when `--trace` is provided. [896](https://github.com/microsoft/binskim/pull/896)
+* BUG: Fix `--trace` missing supported values from SARIF SDK (`ScanTime`, `RuleScanTime`, `PeakWorkingSet`, `TargetsScanned`, `ResultsSummary`). [896](https://github.com/microsoft/binskim/pull/896)
 * DEP: Update `Microsoft.CodeAnalysis.NetAnalyzers` package from 7.0.0 to 7.0.1 to resolve build warnings. [#903](https://github.com/microsoft/binskim/pull/903)
 
 ## **v4.1.0**

docs/UserGuide.md

@@ -41,7 +41,7 @@ The **`analyze`** command supports the following additional arguments:
 
 | Argument (short form, long form) | Meaning |
 | -------------------------------- | ------- |
-| **`--trace`** | Execution traces, expressed as a semicolon-delimited list enclosed in double quotes, that should be emitted to the console and log file (if appropriate). Valid values: PdbLoad. |
+| **`--trace`** | Execution traces, expressed as a semicolon-delimited list enclosed in double quotes, that should be emitted to the console and log file (if appropriate). Valid values: PdbLoad, ScanTime, RuleScanTime, PeakWorkingSet, TargetsScanned, ResultsSummary. |
 | **`--sympath`** | Symbol paths, expressed as a semicolon-delimited list enclosed in double quotes. (e.g. `SRV http://msdl.microsoft.com/download/symbols or Cache d:\symbols;Srv http://symweb`) |
 | **`--local-symbol-directories`** | Local directory paths, expressed as a semicolon-delimited list enclosed in double quotes, that will be examined when attempting to locate PDBs. |
 | **`-o, --output`** | File path used to write and output analysis using [SARIF](https://github.com/Microsoft/sarif-sdk) |

src/BinSkim.Driver/AnalyzeOptions.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 using CommandLine;
 
@@ -13,14 +14,23 @@ namespace Microsoft.CodeAnalysis.IL
     [Verb("analyze", HelpText = "Analyze one or more binary files for security and correctness issues.")]
     public class AnalyzeOptions : AnalyzeOptionsBase
     {
+        private IEnumerable<string> trace = Array.Empty<string>();
         [Option(
             "trace",
             Separator = ';',
             Default = new string[] { },
             HelpText = "Execution traces, expressed as a semicolon-delimited list enclosed in double quotes, " +
                        "that should be emitted to the console and log file (if appropriate). " +
-                       "Valid values: PdbLoad.")]
-        public new IEnumerable<string> Traces { get; set; } = Array.Empty<string>();
+                       "Valid values: PdbLoad, ScanTime, RuleScanTime, PeakWorkingSet, TargetsScanned, ResultsSummary.")]
+        public new IEnumerable<string> Trace
+        {
+            get => this.trace;
+            set
+            {
+                this.trace = value;
+                base.Trace = value?.Where(s => s != nameof(IL.Traces.PdbLoad));
+            }
+        }
 
         [Option(
             "sympath",

src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs

@@ -71,7 +71,7 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze
             context.SymbolPath = options.SymbolsPath;
             context.IgnorePdbLoadError = options.IgnorePdbLoadError;
             context.LocalSymbolDirectories = options.LocalSymbolDirectories;
-            context.TracePdbLoads = options.Traces.Contains(nameof(Traces.PdbLoad));
+            context.TracePdbLoads = options.Trace.Contains(nameof(Traces.PdbLoad));
 
             context.CompilerDataLogger =
                 new CompilerDataLogger(context.OutputFilePath,
@@ -171,7 +171,7 @@ public override int Run(AnalyzeOptions analyzeOptions)
         {
             Stopwatch stopwatch = null;
 
-            if (analyzeOptions.Traces.Where(s => s == "ScanTime").Any())
+            if (analyzeOptions.Trace.Where(s => s == nameof(DefaultTraces.ScanTime)).Any())
             {
                 stopwatch = Stopwatch.StartNew();
             }

src/Test.FunctionalTests.BinSkim.Driver/AnalyzeCommandTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -77,12 +78,15 @@ public void AnalyzeCommand_ShouldThrowWithVersionOne()
         }
 
         [Fact]
-        public void AnalyzeCommand_ShouldNotThrowWithPdbLoadTrace()
+        public void AnalyzeCommand_ShouldNotThrowWithSupportedTrace()
         {
+            IEnumerable<string> allSupportedDefaultTraces = Enum.GetValues(typeof(DefaultTraces)).Cast<DefaultTraces>()
+                                                            .Where(e => e != DefaultTraces.None)
+                                                            .Select(e => e.ToString());
             var options = new AnalyzeOptions
             {
                 TargetFileSpecifiers = new string[] { GetThisTestAssemblyFilePath() },
-                Traces = new[] { "PdbLoad" },
+                Trace = allSupportedDefaultTraces.Append(nameof(Traces.PdbLoad))
             };
 
             var command = new MultithreadedAnalyzeCommand
@@ -92,6 +96,8 @@ public void AnalyzeCommand_ShouldNotThrowWithPdbLoadTrace()
 
             BinaryAnalyzerContext context = null;
             int result = command.Run(options, ref context);
+            context.Traces.Should().HaveCount(allSupportedDefaultTraces.Count());
+            context.TracePdbLoads.Should().BeTrue();
             context.RuntimeExceptions.Should().BeNull();
             result.Should().Be(0);
         }

src/Test.FunctionalTests.BinSkim.Driver/BaselineTests.cs

@@ -438,7 +438,6 @@ private SarifLog RunRules(StringBuilder sb, string inputFileName, Sarif.SarifVer
                 ConfigurationFilePath = "default",
                 SarifOutputVersion = Sarif.SarifVersion.Current,
                 TargetFileSpecifiers = new string[] { inputFileName },
-                Traces = Array.Empty<string>(),
                 Level = new List<FailureLevel> { FailureLevel.Error, FailureLevel.Warning, FailureLevel.Note },
                 Kind = new List<ResultKind> { ResultKind.Fail, ResultKind.Pass },
             };