resolve other comments

microsoft · Jul 19, 2023 · c3d9a8c · c3d9a8c
1 parent 74a7ba8
commit c3d9a8c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 20 deletions.
diff --git a/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs b/src/BinSkim.Driver/MultithreadedAnalyzeCommand.cs
@@ -68,9 +68,9 @@ public override BinaryAnalyzerContext InitializeGlobalContextFromOptions(Analyze
             base.InitializeGlobalContextFromOptions(options, ref context);
 
             // Update context object based on command-line parameters.
-            context.SymbolPath = options.SymbolsPath;
+            context.SymbolPath = options.SymbolsPath ?? context.SymbolPath;
             context.IgnorePdbLoadError = options.IgnorePdbLoadError != null ? options.IgnorePdbLoadError.Value : context.IgnorePdbLoadError;
-            context.LocalSymbolDirectories = options.LocalSymbolDirectories;
+            context.LocalSymbolDirectories = options.LocalSymbolDirectories ?? context.LocalSymbolDirectories;
             context.TracePdbLoads = options.Trace.Contains(nameof(Traces.PdbLoad));
 
             context.CompilerDataLogger =

diff --git a/src/BinSkim.Sdk/BinaryAnalyzerContext.cs b/src/BinSkim.Sdk/BinaryAnalyzerContext.cs
@@ -38,7 +38,11 @@ public override bool IsValidAnalysisTarget
             get => this.Binary?.Valid == true;
         }
 
-        public string LocalSymbolDirectories { get; set; }
+        public string LocalSymbolDirectories
+        {
+            get => this.Policy?.GetProperty(BinaryParsersProperties.LocalSymbolDirectories);
+            set => this.Policy.SetProperty(BinaryParsersProperties.LocalSymbolDirectories, value);
+        }
 
         public bool ComprehensiveBinaryParsing
         {
@@ -48,7 +52,11 @@ public bool ComprehensiveBinaryParsing
 
         public bool TracePdbLoads { get; set; }
 
-        public string SymbolPath { get; set; }
+        public string SymbolPath
+        {
+            get => this.Policy?.GetProperty(BinaryParsersProperties.SymbolPath);
+            set => this.Policy.SetProperty(BinaryParsersProperties.SymbolPath, value);
+        }
 
         public override IAnalysisLogger Logger { get; set; }
 
@@ -66,16 +74,7 @@ public override string MimeType
 
         public override bool AnalysisComplete { get; set; }
 
-        public CompilerDataLogger CompilerDataLogger
-        {
-            get
-            {
-                return this.Policy != null
-                    ? this.Policy.GetProperty(SharedCompilerDataLoggerProperty)
-                    : null;
-            }
-            set { this.Policy.SetProperty(SharedCompilerDataLoggerProperty, value); }
-        }
+        public CompilerDataLogger CompilerDataLogger { get; set; }
 
         public bool IgnorePdbLoadError
         {
@@ -110,11 +109,6 @@ protected virtual void Dispose(bool disposing)
             }
         }
 
-        public static PerLanguageOption<CompilerDataLogger> SharedCompilerDataLoggerProperty { get; } =
-            new PerLanguageOption<CompilerDataLogger>(
-                "CompilerTelemetry", nameof(SharedCompilerDataLoggerProperty), defaultValue: () => null,
-                "A shared CompilerDataLogger instance that will be passed to all skimmers.");
-
         public override void Dispose()
         {
             // Do not change this code. Put cleanup code in Dispose(bool disposing) above.

diff --git a/src/BinaryParsers/BinaryParsersProperties.cs b/src/BinaryParsers/BinaryParsersProperties.cs
@@ -17,7 +17,9 @@ public IEnumerable<IOption> GetOptions()
             {
                 ComprehensiveBinaryParsing,
                 IgnorePdbLoadError,
-                IncludeWixBinaries
+                IncludeWixBinaries,
+                LocalSymbolDirectories,
+                SymbolPath
             }.ToImmutableArray();
         }
 
@@ -36,5 +38,21 @@ public IEnumerable<IOption> GetOptions()
             new PerLanguageOption<bool>(
                 "BinaryParsers", nameof(IncludeWixBinaries), defaultValue: () => false,
                 "Set this value to 'true' to include Wix binaries in the analysis.");
+
+        public static PerLanguageOption<string> LocalSymbolDirectories { get; } =
+            new PerLanguageOption<string>(
+                "BinaryParsers", nameof(LocalSymbolDirectories), defaultValue: () => null,
+                "A set of semicolon-delimited local directory paths that will be examined when attempting to locate PDBs.");
+
+        public static PerLanguageOption<string> SymbolPath { get; } =
+            new PerLanguageOption<string>(
+                "BinaryParsers", nameof(SymbolPath), defaultValue: () => null,
+                "Symbols path value, e.g., Cache*c:\\symbols;SRV*https://msdl.microsoft.com/download/symbols " +
+                "or Cache*d:\\symbols;Srv*https://symweb. See " +
+                "https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/advanced-symsrv-use for " +
+                "syntax information. Note that BinSkim will clear the _NT_SYMBOL_PATH and _NT_ALT_SYMBOL_PATH " +
+                "environment variables at runtime. Use this argument instead for specifying the symbol path." +
+                "WARNING: Be sure to specify a local file cache in the symbol path if at all possible, in order " +
+                "to avoid the possibility of significance time-to-analyze performance degradataion.");
     }
 }