diff --git a/.gitignore b/.gitignore index 1768ebb161fd..c0e9ed803ff6 100644 --- a/.gitignore +++ b/.gitignore @@ -16,5 +16,6 @@ # It's useful (though not required) to be able to unpack codeql in the ql checkout itself /codeql/ -.vscode/settings.json + csharp/extractor/Semmle.Extraction.CSharp.Driver/Properties/launchSettings.json +.vscode diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/AssemblyCache.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/AssemblyCache.cs index db2664bf4c94..f93911b8a383 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/AssemblyCache.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/AssemblyCache.cs @@ -163,7 +163,19 @@ public AssemblyInfo ResolveReference(string id) /// /// The filename to query. /// The assembly info. - public AssemblyInfo GetAssemblyInfo(string filepath) => assemblyInfo[filepath]; + public AssemblyInfo GetAssemblyInfo(string filepath) + { + if(assemblyInfo.TryGetValue(filepath, out var info)) + { + return info; + } + else + { + info = AssemblyInfo.ReadFromFile(filepath); + assemblyInfo.Add(filepath, info); + return info; + } + } // List of pending DLLs to index. readonly List dlls = new List(); diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs index b0ef328bcd00..2894222ca891 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/BuildAnalysis.cs @@ -1,10 +1,13 @@ -using System; +using Semmle.Util; +using System; using System.Collections.Generic; using System.IO; using System.Linq; -using System.Runtime.InteropServices; -using Semmle.Util; using Semmle.Extraction.CSharp.Standalone; +using System.Threading.Tasks; +using System.Collections.Concurrent; +using System.Text; +using System.Security.Cryptography; namespace Semmle.BuildAnalyser { @@ -43,19 +46,18 @@ interface IBuildAnalysis /// /// Main implementation of the build analysis. /// - class BuildAnalysis : IBuildAnalysis + class BuildAnalysis : IBuildAnalysis, IDisposable { - readonly AssemblyCache assemblyCache; - readonly NugetPackages nuget; - readonly IProgressMonitor progressMonitor; - HashSet usedReferences = new HashSet(); - readonly HashSet usedSources = new HashSet(); - readonly HashSet missingSources = new HashSet(); - readonly Dictionary unresolvedReferences = new Dictionary(); - readonly DirectoryInfo sourceDir; - int failedProjects, succeededProjects; - readonly string[] allSources; - int conflictedReferences = 0; + private readonly AssemblyCache assemblyCache; + private readonly NugetPackages nuget; + private readonly IProgressMonitor progressMonitor; + private readonly IDictionary usedReferences = new ConcurrentDictionary(); + private readonly IDictionary sources = new ConcurrentDictionary(); + private readonly IDictionary unresolvedReferences = new ConcurrentDictionary(); + private readonly DirectoryInfo sourceDir; + private int failedProjects, succeededProjects; + private readonly string[] allSources; + private int conflictedReferences = 0; /// /// Performs a C# build analysis. @@ -64,6 +66,8 @@ class BuildAnalysis : IBuildAnalysis /// Display of analysis progress. public BuildAnalysis(Options options, IProgressMonitor progress) { + var startTime = DateTime.Now; + progressMonitor = progress; sourceDir = new DirectoryInfo(options.SrcDir); @@ -74,36 +78,43 @@ public BuildAnalysis(Options options, IProgressMonitor progress) Where(d => !options.ExcludesFile(d)). ToArray(); - var dllDirNames = options.DllDirs.Select(Path.GetFullPath); + var dllDirNames = options.DllDirs.Select(Path.GetFullPath).ToList(); + PackageDirectory = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName)); if (options.UseNuGet) { - nuget = new NugetPackages(sourceDir.FullName); - ReadNugetFiles(); - dllDirNames = dllDirNames.Concat(Enumerators.Singleton(nuget.PackageDirectory)); + try + { + nuget = new NugetPackages(sourceDir.FullName, PackageDirectory); + ReadNugetFiles(); + } + catch(FileNotFoundException) + { + progressMonitor.MissingNuGet(); + } } // Find DLLs in the .Net Framework if (options.ScanNetFrameworkDlls) { - dllDirNames = dllDirNames.Concat(Runtime.Runtimes.Take(1)); + dllDirNames.Add(Runtime.Runtimes.First()); } - assemblyCache = new BuildAnalyser.AssemblyCache(dllDirNames, progress); - - // Analyse all .csproj files in the source tree. - if (options.SolutionFile != null) - { - AnalyseSolution(options.SolutionFile); - } - else if (options.AnalyseCsProjFiles) + // These files can sometimes prevent `dotnet restore` from working correctly. + using (new FileRenamer(sourceDir.GetFiles("global.json", SearchOption.AllDirectories))) + using (new FileRenamer(sourceDir.GetFiles("Directory.Build.props", SearchOption.AllDirectories))) { - AnalyseProjectFiles(); - } + var solutions = options.SolutionFile != null ? + new[] { options.SolutionFile } : + sourceDir.GetFiles("*.sln", SearchOption.AllDirectories).Select(d => d.FullName); - if (!options.AnalyseCsProjFiles) - { - usedReferences = new HashSet(assemblyCache.AllAssemblies.Select(a => a.Filename)); + RestoreSolutions(solutions); + dllDirNames.Add(PackageDirectory.DirInfo.FullName); + assemblyCache = new BuildAnalyser.AssemblyCache(dllDirNames, progress); + AnalyseSolutions(solutions); + + foreach (var filename in assemblyCache.AllAssemblies.Select(a => a.Filename)) + UseReference(filename); } ResolveConflicts(); @@ -114,7 +125,7 @@ public BuildAnalysis(Options options, IProgressMonitor progress) } // Output the findings - foreach (var r in usedReferences) + foreach (var r in usedReferences.Keys) { progressMonitor.ResolvedReference(r); } @@ -132,7 +143,27 @@ public BuildAnalysis(Options options, IProgressMonitor progress) UnresolvedReferences.Count(), conflictedReferences, succeededProjects + failedProjects, - failedProjects); + failedProjects, + DateTime.Now - startTime); + } + + /// + /// Computes a unique temp directory for the packages associated + /// with this source tree. Use a SHA1 of the directory name. + /// + /// + /// The full path of the temp directory. + private static string ComputeTempDirectory(string srcDir) + { + var bytes = Encoding.Unicode.GetBytes(srcDir); + + using var sha1 = new SHA1CryptoServiceProvider(); + var sha = sha1.ComputeHash(bytes); + var sb = new StringBuilder(); + foreach (var b in sha.Take(8)) + sb.AppendFormat("{0:x2}", b); + + return Path.Combine(Path.GetTempPath(), "GitHub", "packages", sb.ToString()); } /// @@ -143,7 +174,7 @@ public BuildAnalysis(Options options, IProgressMonitor progress) void ResolveConflicts() { var sortedReferences = usedReferences. - Select(r => assemblyCache.GetAssemblyInfo(r)). + Select(r => assemblyCache.GetAssemblyInfo(r.Key)). OrderBy(r => r.Version). ToArray(); @@ -154,7 +185,9 @@ void ResolveConflicts() finalAssemblyList[r.Name] = r; // Update the used references list - usedReferences = new HashSet(finalAssemblyList.Select(r => r.Value.Filename)); + usedReferences.Clear(); + foreach (var r in finalAssemblyList.Select(r => r.Value.Filename)) + UseReference(r); // Report the results foreach (var r in sortedReferences) @@ -183,7 +216,7 @@ void ReadNugetFiles() /// The filename of the reference. void UseReference(string reference) { - usedReferences.Add(reference); + usedReferences[reference] = true; } /// @@ -192,25 +225,18 @@ void UseReference(string reference) /// The source file. void UseSource(FileInfo sourceFile) { - if (sourceFile.Exists) - { - usedSources.Add(sourceFile.FullName); - } - else - { - missingSources.Add(sourceFile.FullName); - } + sources[sourceFile.FullName] = sourceFile.Exists; } /// /// The list of resolved reference files. /// - public IEnumerable ReferenceFiles => this.usedReferences; + public IEnumerable ReferenceFiles => this.usedReferences.Keys; /// /// The list of source files used in projects. /// - public IEnumerable ProjectSourceFiles => usedSources; + public IEnumerable ProjectSourceFiles => sources.Where(s => s.Value).Select(s => s.Key); /// /// All of the source files in the source directory. @@ -226,7 +252,7 @@ void UseSource(FileInfo sourceFile) /// List of source files which were mentioned in project files but /// do not exist on the file system. /// - public IEnumerable MissingSourceFiles => missingSources; + public IEnumerable MissingSourceFiles => sources.Where(s => !s.Value).Select(s => s.Key); /// /// Record that a particular reference couldn't be resolved. @@ -239,74 +265,101 @@ void UnresolvedReference(string id, string projectFile) unresolvedReferences[id] = projectFile; } - /// - /// Performs an analysis of all .csproj files. - /// - void AnalyseProjectFiles() - { - AnalyseProjectFiles(sourceDir.GetFiles("*.csproj", SearchOption.AllDirectories)); - } + readonly TemporaryDirectory PackageDirectory; /// /// Reads all the source files and references from the given list of projects. /// /// The list of projects to analyse. - void AnalyseProjectFiles(FileInfo[] projectFiles) + void AnalyseProjectFiles(IEnumerable projectFiles) { - progressMonitor.AnalysingProjectFiles(projectFiles.Count()); - foreach (var proj in projectFiles) + AnalyseProject(proj); + } + + void AnalyseProject(FileInfo project) + { + if(!project.Exists) { - try - { - var csProj = new CsProjFile(proj); + progressMonitor.MissingProject(project.FullName); + return; + } + + try + { + var csProj = new CsProjFile(project); - foreach (var @ref in csProj.References) + foreach (var @ref in csProj.References) + { + AssemblyInfo resolved = assemblyCache.ResolveReference(@ref); + if (!resolved.Valid) { - AssemblyInfo resolved = assemblyCache.ResolveReference(@ref); - if (!resolved.Valid) - { - UnresolvedReference(@ref, proj.FullName); - } - else - { - UseReference(resolved.Filename); - } + UnresolvedReference(@ref, project.FullName); } - - foreach (var src in csProj.Sources) + else { - // Make a note of which source files the projects use. - // This information doesn't affect the build but is dumped - // as diagnostic output. - UseSource(new FileInfo(src)); + UseReference(resolved.Filename); } - ++succeededProjects; } - catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] + + foreach (var src in csProj.Sources) { - ++failedProjects; - progressMonitor.FailedProjectFile(proj.FullName, ex.Message); + // Make a note of which source files the projects use. + // This information doesn't affect the build but is dumped + // as diagnostic output. + UseSource(new FileInfo(src)); } + + ++succeededProjects; + } + catch (Exception ex) // lgtm[cs/catch-of-all-exceptions] + { + ++failedProjects; + progressMonitor.FailedProjectFile(project.FullName, ex.Message); } + } - /// - /// Delete packages directory. - /// - public void Cleanup() + void Restore(string projectOrSolution) + { + int exit = DotNet.RestoreToDirectory(projectOrSolution, PackageDirectory.DirInfo.FullName); + switch(exit) + { + case 0: + case 1: + // No errors + break; + default: + progressMonitor.CommandFailed("dotnet", $"restore \"{projectOrSolution}\"", exit); + break; + } + } + + public void RestoreSolutions(IEnumerable solutions) { - if (nuget != null) nuget.Cleanup(progressMonitor); + Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = 4 }, Restore); } - /// - /// Analyse all project files in a given solution only. - /// - /// The filename of the solution. - public void AnalyseSolution(string solutionFile) + public void AnalyseSolutions(IEnumerable solutions) + { + Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = 4 } , solutionFile => + { + try + { + var sln = new SolutionFile(solutionFile); + progressMonitor.AnalysingSolution(solutionFile); + AnalyseProjectFiles(sln.Projects.Select(p => new FileInfo(p)).Where(p => p.Exists)); + } + catch (Microsoft.Build.Exceptions.InvalidProjectFileException ex) + { + progressMonitor.FailedProjectFile(solutionFile, ex.BaseMessage); + } + }); + } + + public void Dispose() { - var sln = new SolutionFile(solutionFile); - AnalyseProjectFiles(sln.Projects.Select(p => new FileInfo(p)).ToArray()); + PackageDirectory?.Dispose(); } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/CsProjFile.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/CsProjFile.cs index 2c9e72c1eaa8..1083c9b6257c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/CsProjFile.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/CsProjFile.cs @@ -10,12 +10,18 @@ namespace Semmle.BuildAnalyser /// class CsProjFile { + private string Filename { get; } + + private string Directory => Path.GetDirectoryName(Filename); + /// /// Reads the .csproj file. /// /// The .csproj file. public CsProjFile(FileInfo filename) { + Filename = filename.FullName; + try { // This can fail if the .csproj is invalid or has @@ -39,7 +45,7 @@ public CsProjFile(FileInfo filename) /// and there seems to be no way to make it succeed. Fails on Linux. /// /// The file to read. - public void ReadMsBuildProject(FileInfo filename) + private void ReadMsBuildProject(FileInfo filename) { var msbuildProject = new Microsoft.Build.Execution.ProjectInstance(filename.FullName); @@ -62,7 +68,7 @@ public void ReadMsBuildProject(FileInfo filename) /// fallback if ReadMsBuildProject() fails. /// /// The .csproj file. - public void ReadProjectFileAsXml(FileInfo filename) + private void ReadProjectFileAsXml(FileInfo filename) { var projFile = new XmlDocument(); var mgr = new XmlNamespaceManager(projFile.NameTable); @@ -71,22 +77,48 @@ public void ReadProjectFileAsXml(FileInfo filename) var projDir = filename.Directory; var root = projFile.DocumentElement; - references = - root.SelectNodes("/msbuild:Project/msbuild:ItemGroup/msbuild:Reference/@Include", mgr). - NodeList(). - Select(node => node.Value). - ToArray(); + // Figure out if it's dotnet core - var relativeCsIncludes = - root.SelectNodes("/msbuild:Project/msbuild:ItemGroup/msbuild:Compile/@Include", mgr). - NodeList(). - Select(node => node.Value). - ToArray(); + bool netCoreProjectFile = root.GetAttribute("Sdk") == "Microsoft.NET.Sdk"; - csFiles = relativeCsIncludes. - Select(cs => Path.DirectorySeparatorChar == '/' ? cs.Replace("\\", "/") : cs). - Select(f => Path.GetFullPath(Path.Combine(projDir.FullName, f))). - ToArray(); + if (netCoreProjectFile) + { + var relativeCsIncludes = + root.SelectNodes("/Project/ItemGroup/Compile/@Include", mgr). + NodeList(). + Select(node => node.Value). + ToArray(); + + var explicitCsFiles = relativeCsIncludes. + Select(cs => Path.DirectorySeparatorChar == '/' ? cs.Replace("\\", "/") : cs). + Select(f => Path.GetFullPath(Path.Combine(projDir.FullName, f))); + + var additionalCsFiles = System.IO.Directory.GetFiles(Directory, "*.cs", SearchOption.AllDirectories); + + csFiles = explicitCsFiles.Concat(additionalCsFiles).ToArray(); + + references = new string[0]; + } + else + { + + references = + root.SelectNodes("/msbuild:Project/msbuild:ItemGroup/msbuild:Reference/@Include", mgr). + NodeList(). + Select(node => node.Value). + ToArray(); + + var relativeCsIncludes = + root.SelectNodes("/msbuild:Project/msbuild:ItemGroup/msbuild:Compile/@Include", mgr). + NodeList(). + Select(node => node.Value). + ToArray(); + + csFiles = relativeCsIncludes. + Select(cs => Path.DirectorySeparatorChar == '/' ? cs.Replace("\\", "/") : cs). + Select(f => Path.GetFullPath(Path.Combine(projDir.FullName, f))). + ToArray(); + } } string[] references; diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/DotNet.cs new file mode 100644 index 000000000000..6edd217af8dc --- /dev/null +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/DotNet.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; + +namespace Semmle.BuildAnalyser +{ + /// + /// Utilities to run the "dotnet" command. + /// + static class DotNet + { + public static int RestoreToDirectory(string projectOrSolutionFile, string packageDirectory) + { + using var proc = Process.Start("dotnet", $"restore --no-dependencies \"{projectOrSolutionFile}\" --packages \"{packageDirectory}\" /p:DisableImplicitNuGetFallbackFolder=true"); + proc.WaitForExit(); + return proc.ExitCode; + } + } +} diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/NugetPackages.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/NugetPackages.cs index 1f0755f307f4..2ea3afb6c691 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/NugetPackages.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/NugetPackages.cs @@ -1,10 +1,9 @@ -using System; +using Semmle.Util; +using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Linq; -using System.Security.Cryptography; -using System.Text; namespace Semmle.BuildAnalyser { @@ -19,10 +18,10 @@ class NugetPackages /// Create the package manager for a specified source tree. /// /// The source directory. - public NugetPackages(string sourceDir) + public NugetPackages(string sourceDir, TemporaryDirectory packageDirectory) { SourceDirectory = sourceDir; - PackageDirectory = computeTempDirectory(sourceDir); + PackageDirectory = packageDirectory; // Expect nuget.exe to be in a `nuget` directory under the directory containing this exe. var currentAssembly = System.Reflection.Assembly.GetExecutingAssembly().Location; @@ -50,45 +49,12 @@ public void FindPackages() /// public IEnumerable PackageFiles => packages; - // Whether to delete the packages directory prior to each run. - // Makes each build more reproducible. - const bool cleanupPackages = true; - - public void Cleanup(IProgressMonitor pm) - { - var packagesDirectory = new DirectoryInfo(PackageDirectory); - - if (packagesDirectory.Exists) - { - try - { - packagesDirectory.Delete(true); - } - catch (System.IO.IOException ex) - { - pm.Warning(string.Format("Couldn't delete package directory - it's probably held open by something else: {0}", ex.Message)); - } - } - } - /// /// Download the packages to the temp folder. /// /// The progress monitor used for reporting errors etc. public void InstallPackages(IProgressMonitor pm) { - if (cleanupPackages) - { - Cleanup(pm); - } - - var packagesDirectory = new DirectoryInfo(PackageDirectory); - - if (!Directory.Exists(PackageDirectory)) - { - packagesDirectory.Create(); - } - foreach (var package in packages) { RestoreNugetPackage(package.FullName, pm); @@ -109,31 +75,7 @@ public string SourceDirectory /// This will be in the Temp location /// so as to not trample the source tree. /// - public string PackageDirectory - { - get; - private set; - } - - readonly SHA1CryptoServiceProvider sha1 = new SHA1CryptoServiceProvider(); - - /// - /// Computes a unique temp directory for the packages associated - /// with this source tree. Use a SHA1 of the directory name. - /// - /// - /// The full path of the temp directory. - string computeTempDirectory(string srcDir) - { - var bytes = Encoding.Unicode.GetBytes(srcDir); - - var sha = sha1.ComputeHash(bytes); - var sb = new StringBuilder(); - foreach (var b in sha.Take(8)) - sb.AppendFormat("{0:x2}", b); - - return Path.Combine(Path.GetTempPath(), "Semmle", "packages", sb.ToString()); - } + public TemporaryDirectory PackageDirectory { get; } /// /// Restore all files in a specified package. @@ -171,16 +113,15 @@ void RestoreNugetPackage(string package, IProgressMonitor pm) try { - using (var p = Process.Start(pi)) + using var p = Process.Start(pi); + + string output = p.StandardOutput.ReadToEnd(); + string error = p.StandardError.ReadToEnd(); + + p.WaitForExit(); + if (p.ExitCode != 0) { - string output = p.StandardOutput.ReadToEnd(); - string error = p.StandardError.ReadToEnd(); - - p.WaitForExit(); - if (p.ExitCode != 0) - { - pm.FailedNugetCommand(pi.FileName, pi.Arguments, output + error); - } + pm.FailedNugetCommand(pi.FileName, pi.Arguments, output + error); } } catch (Exception ex) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Program.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Program.cs index e0367fa63c13..106771faef2f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Program.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Program.cs @@ -23,7 +23,7 @@ public Extraction(string directory) /// /// Searches for source/references and creates separate extractions. /// - class Analysis + class Analysis : IDisposable { readonly ILogger logger; @@ -71,12 +71,9 @@ public void AnalyseProjects(Options options) projectExtraction.Sources.AddRange(options.SolutionFile == null ? buildAnalysis.AllSourceFiles : buildAnalysis.ProjectSourceFiles); } - /// - /// Delete any Nuget assemblies. - /// - public void Cleanup() + public void Dispose() { - buildAnalysis.Cleanup(); + buildAnalysis.Dispose(); } }; @@ -85,8 +82,9 @@ public class Program static int Main(string[] args) { var options = Options.Create(args); + // options.CIL = true; // To do: Enable this var output = new ConsoleLogger(options.Verbosity); - var a = new Analysis(output); + using var a = new Analysis(output); if (options.Help) { @@ -97,6 +95,8 @@ static int Main(string[] args) if (options.Errors) return 1; + var start = DateTime.Now; + output.Log(Severity.Info, "Running C# standalone extractor"); a.AnalyseProjects(options); int sourceFiles = a.Extraction.Sources.Count(); @@ -117,10 +117,9 @@ static int Main(string[] args) new ExtractionProgress(output), new FileLogger(options.Verbosity, Extractor.GetCSharpLogPath()), options); - output.Log(Severity.Info, "Extraction complete"); + output.Log(Severity.Info, $"Extraction completed in {DateTime.Now-start}"); } - a.Cleanup(); return 0; } @@ -151,7 +150,7 @@ public void MissingNamespace(string @namespace) public void MissingSummary(int missingTypes, int missingNamespaces) { - logger.Log(Severity.Info, "Failed to resolve {0} types and {1} namespaces", missingTypes, missingNamespaces); + logger.Log(Severity.Info, "Failed to resolve {0} types in {1} namespaces", missingTypes, missingNamespaces); } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/ProgressMonitor.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/ProgressMonitor.cs index f4bde55ec557..5bbe1e3a5b28 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/ProgressMonitor.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/ProgressMonitor.cs @@ -1,4 +1,5 @@ using Semmle.Util.Logging; +using System; namespace Semmle.BuildAnalyser { @@ -9,15 +10,17 @@ interface IProgressMonitor { void FindingFiles(string dir); void UnresolvedReference(string id, string project); - void AnalysingProjectFiles(int count); + void AnalysingSolution(string filename); void FailedProjectFile(string filename, string reason); void FailedNugetCommand(string exe, string args, string message); void NugetInstall(string package); void ResolvedReference(string filename); - void Summary(int existingSources, int usedSources, int missingSources, int references, int unresolvedReferences, int resolvedConflicts, int totalProjects, int failedProjects); + void Summary(int existingSources, int usedSources, int missingSources, int references, int unresolvedReferences, int resolvedConflicts, int totalProjects, int failedProjects, TimeSpan analysisTime); void Warning(string message); void ResolvedConflict(string asm1, string asm2); void MissingProject(string projectFile); + void CommandFailed(string exe, string arguments, int exitCode); + void MissingNuGet(); } class ProgressMonitor : IProgressMonitor @@ -46,9 +49,9 @@ public void UnresolvedReference(string id, string project) logger.Log(Severity.Debug, "Unresolved {0} referenced by {1}", id, project); } - public void AnalysingProjectFiles(int count) + public void AnalysingSolution(string filename) { - logger.Log(Severity.Info, "Analyzing project files..."); + logger.Log(Severity.Info, $"Analyzing {filename}..."); } public void FailedProjectFile(string filename, string reason) @@ -73,7 +76,9 @@ public void ResolvedReference(string filename) } public void Summary(int existingSources, int usedSources, int missingSources, - int references, int unresolvedReferences, int resolvedConflicts, int totalProjects, int failedProjects) + int references, int unresolvedReferences, + int resolvedConflicts, int totalProjects, int failedProjects, + TimeSpan analysisTime) { logger.Log(Severity.Info, ""); logger.Log(Severity.Info, "Build analysis summary:"); @@ -85,6 +90,7 @@ public void Summary(int existingSources, int usedSources, int missingSources, logger.Log(Severity.Info, "{0, 6} resolved assembly conflicts", resolvedConflicts); logger.Log(Severity.Info, "{0, 6} projects", totalProjects); logger.Log(Severity.Info, "{0, 6} missing/failed projects", failedProjects); + logger.Log(Severity.Info, "Build analysis completed in {0}", analysisTime); } public void Warning(string message) @@ -94,12 +100,22 @@ public void Warning(string message) public void ResolvedConflict(string asm1, string asm2) { - logger.Log(Severity.Info, "Resolved {0} as {1}", asm1, asm2); + logger.Log(Severity.Debug, "Resolved {0} as {1}", asm1, asm2); } public void MissingProject(string projectFile) { logger.Log(Severity.Info, "Solution is missing {0}", projectFile); } + + public void CommandFailed(string exe, string arguments, int exitCode) + { + logger.Log(Severity.Error, $"Command {exe} {arguments} failed with exit code {exitCode}"); + } + + public void MissingNuGet() + { + logger.Log(Severity.Error, "Missing nuget.exe"); + } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Semmle.Extraction.CSharp.Standalone.csproj b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Semmle.Extraction.CSharp.Standalone.csproj index 4cf0274b7375..f9efd0d9ebbb 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Semmle.Extraction.CSharp.Standalone.csproj +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/Semmle.Extraction.CSharp.Standalone.csproj @@ -1,4 +1,4 @@ - + Exe @@ -14,6 +14,7 @@ + diff --git a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/SolutionFile.cs b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/SolutionFile.cs index b1a3edd4cf6d..b4551dd80243 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.Standalone/SolutionFile.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.Standalone/SolutionFile.cs @@ -12,6 +12,8 @@ class SolutionFile { readonly Microsoft.Build.Construction.SolutionFile solutionFile; + private string FullPath { get; } + /// /// Read the file. /// @@ -19,8 +21,8 @@ class SolutionFile public SolutionFile(string filename) { // SolutionFile.Parse() expects a rooted path. - var fullPath = Path.GetFullPath(filename); - solutionFile = Microsoft.Build.Construction.SolutionFile.Parse(fullPath); + FullPath = Path.GetFullPath(filename); + solutionFile = Microsoft.Build.Construction.SolutionFile.Parse(FullPath); } /// diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Access.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Access.cs index 0488fe84ffe8..6962e8381d9a 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Access.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Access.cs @@ -45,7 +45,10 @@ static ExprKind AccessKind(Context cx, ISymbol symbol) Access(ExpressionNodeInfo info, ISymbol symbol, bool implicitThis, IEntity target) : base(info.SetKind(AccessKind(info.Context, symbol))) { - cx.TrapWriter.Writer.expr_access(this, target); + if (!(target is null)) + { + cx.TrapWriter.Writer.expr_access(this, target); + } if (implicitThis && !symbol.IsStatic) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/MemberAccess.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/MemberAccess.cs index e41ef0edf23a..0bc84ca9c0c3 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/MemberAccess.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/MemberAccess.cs @@ -71,7 +71,9 @@ static Expression Create(ExpressionNodeInfo info, ExpressionSyntax expression, S if (symbol == null) { info.Context.ModelError(info.Node, "Failed to determine symbol for member access"); - return new MemberAccess(info.SetKind(ExprKind.UNKNOWN), expression, symbol); + // Default to property access - this can still give useful results but + // the target of the expression should be checked in QL. + return new MemberAccess(info.SetKind(ExprKind.PROPERTY_ACCESS), expression, symbol); } ExprKind kind; diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/NamespaceDeclaration.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/NamespaceDeclaration.cs index a0dd41aaafb8..7a14bb719fc1 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/NamespaceDeclaration.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/NamespaceDeclaration.cs @@ -23,7 +23,8 @@ public NamespaceDeclaration(Context cx, NamespaceDeclarationSyntax node, Namespa protected override void Populate(TextWriter trapFile) { - var ns = Namespace.Create(cx, (INamespaceSymbol)cx.GetModel(Node).GetSymbolInfo(Node.Name).Symbol); + var @namespace = (INamespaceSymbol) cx.GetModel(Node).GetSymbolInfo(Node.Name).Symbol; + var ns = Namespace.Create(cx, @namespace); trapFile.namespace_declarations(this, ns); trapFile.namespace_declaration_location(this, cx.Create(Node.Name.GetLocation())); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs index e22d32c0d019..cecec5bc0286 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs @@ -25,7 +25,7 @@ public override void Populate(TextWriter trapFile) { if (symbol.TypeKind == TypeKind.Error) { - Context.Extractor.MissingType(symbol.ToString()); + Context.Extractor.MissingType(symbol.ToString(), Context.FromSource); return; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/UsingDirective.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/UsingDirective.cs index 4fdbe7c18ad4..02b67efc1642 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/UsingDirective.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/UsingDirective.cs @@ -32,7 +32,7 @@ protected override void Populate(TextWriter trapFile) if (namespaceSymbol == null) { - cx.Extractor.MissingNamespace(Node.Name.ToFullString()); + cx.Extractor.MissingNamespace(Node.Name.ToFullString(), cx.FromSource); cx.ModelError(Node, "Namespace not found"); return; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs index c2cb6a367eb1..6050ad910a52 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs @@ -214,7 +214,6 @@ private static void BuildAssembly(IAssemblySymbol asm, TextWriter trapFile, bool static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, TextWriter trapFile, Action subTermAction) { bool prefixAssembly = true; - if (cx.Extractor.Standalone) prefixAssembly = false; if (named.ContainingAssembly is null) prefixAssembly = false; if (named.IsTupleType) diff --git a/csharp/extractor/Semmle.Extraction/Context.cs b/csharp/extractor/Semmle.Extraction/Context.cs index 93f993818586..918642f198d8 100644 --- a/csharp/extractor/Semmle.Extraction/Context.cs +++ b/csharp/extractor/Semmle.Extraction/Context.cs @@ -155,7 +155,7 @@ private Entity CreateNonNullEntity(ICachedEntityFactory Scope.FromSource; + public bool IsGlobalContext => Scope.IsGlobalScope; public readonly ICommentGenerator CommentGenerator = new CommentProcessor(); diff --git a/csharp/extractor/Semmle.Extraction/ExtractionScope.cs b/csharp/extractor/Semmle.Extraction/ExtractionScope.cs index 7f4f599fe5c7..60daff8d0138 100644 --- a/csharp/extractor/Semmle.Extraction/ExtractionScope.cs +++ b/csharp/extractor/Semmle.Extraction/ExtractionScope.cs @@ -25,6 +25,8 @@ public interface IExtractionScope bool InFileScope(string path); bool IsGlobalScope { get; } + + bool FromSource { get; } } /// @@ -49,6 +51,8 @@ public AssemblyScope(IAssemblySymbol symbol, string path, bool isOutput) public bool InScope(ISymbol symbol) => SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly) || SymbolEqualityComparer.Default.Equals(symbol, assembly); + + public bool FromSource => false; } /// @@ -68,5 +72,7 @@ public SourceScope(SyntaxTree tree) public bool InFileScope(string path) => path == sourceTree.FilePath; public bool InScope(ISymbol symbol) => symbol.Locations.Any(loc => loc.SourceTree == sourceTree); + + public bool FromSource => true; } } diff --git a/csharp/extractor/Semmle.Extraction/Extractor.cs b/csharp/extractor/Semmle.Extraction/Extractor.cs index e470d3258ece..13750c1aa5c8 100644 --- a/csharp/extractor/Semmle.Extraction/Extractor.cs +++ b/csharp/extractor/Semmle.Extraction/Extractor.cs @@ -50,13 +50,15 @@ public interface IExtractor /// Record a new error type. /// /// The display name of the type, qualified where possible. - void MissingType(string fqn); + /// If the missing type was referenced from a source file. + void MissingType(string fqn, bool fromSource); /// /// Record an unresolved `using namespace` directive. /// /// The full name of the namespace. - void MissingNamespace(string fqn); + /// If the missing namespace was referenced from a source file. + void MissingNamespace(string fqn, bool fromSource); /// /// The list of missing types. @@ -167,16 +169,22 @@ public int Errors readonly ISet missingTypes = new SortedSet(); readonly ISet missingNamespaces = new SortedSet(); - public void MissingType(string fqn) + public void MissingType(string fqn, bool fromSource) { - lock (mutex) - missingTypes.Add(fqn); + if (fromSource) + { + lock (mutex) + missingTypes.Add(fqn); + } } - public void MissingNamespace(string fqdn) + public void MissingNamespace(string fqdn, bool fromSource) { - lock (mutex) - missingNamespaces.Add(fqdn); + if (fromSource) + { + lock (mutex) + missingNamespaces.Add(fqdn); + } } public Context CreateContext(Compilation c, TrapWriter trapWriter, IExtractionScope scope) diff --git a/csharp/extractor/Semmle.Util/FileRenamer.cs b/csharp/extractor/Semmle.Util/FileRenamer.cs new file mode 100644 index 000000000000..ad5001f7e135 --- /dev/null +++ b/csharp/extractor/Semmle.Util/FileRenamer.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; + +namespace Semmle.Util +{ + /// + /// Utility to temporarily rename a set of files. + /// + public sealed class FileRenamer : IDisposable + { + readonly string[] files; + const string suffix = ".codeqlhidden"; + + public FileRenamer(IEnumerable oldFiles) + { + files = oldFiles.Select(f => f.FullName).ToArray(); + + foreach (var file in files) + { + File.Move(file, file + suffix); + } + } + + public void Dispose() + { + foreach (var file in files) + { + File.Move(file + suffix, file); + } + } + } +} diff --git a/csharp/extractor/Semmle.Util/TemporaryDirectory.cs b/csharp/extractor/Semmle.Util/TemporaryDirectory.cs new file mode 100644 index 000000000000..a155372c9d89 --- /dev/null +++ b/csharp/extractor/Semmle.Util/TemporaryDirectory.cs @@ -0,0 +1,30 @@ +using System; +using System.IO; +using System.Linq; +using System.Security.Cryptography; +using System.Text; + +namespace Semmle.Util +{ + /// + /// A temporary directory that is created within the system temp directory. + /// When this object is disposed, the directory is deleted. + /// + public sealed class TemporaryDirectory : IDisposable + { + public DirectoryInfo DirInfo { get; } + + public TemporaryDirectory(string name) + { + DirInfo = new DirectoryInfo(name); + DirInfo.Create(); + } + + public void Dispose() + { + DirInfo.Delete(true); + } + + public override string ToString() => DirInfo.FullName.ToString(); + } +} diff --git a/csharp/ql/src/semmle/code/csharp/exprs/Access.qll b/csharp/ql/src/semmle/code/csharp/exprs/Access.qll index c0d75bfd9fe6..d1e1017e0d15 100644 --- a/csharp/ql/src/semmle/code/csharp/exprs/Access.qll +++ b/csharp/ql/src/semmle/code/csharp/exprs/Access.qll @@ -388,7 +388,12 @@ library class PropertyAccessExpr extends Expr, @property_access_expr { /** Gets the target of this property access. */ Property getProperty() { expr_access(this, result) } - override string toString() { result = "access to property " + this.getProperty().getName() } + override string toString() { + result = "access to property " + this.getProperty().getName() + or + not exists(this.getProperty()) and + result = "access to property (unknown)" + } } /** diff --git a/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected b/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected index 06473196df99..a9eb533c1522 100644 --- a/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected +++ b/csharp/ql/test/library-tests/standalone/controlflow/cfg.expected @@ -7,9 +7,14 @@ | ControlFlow.cs:10:9:10:43 | Call (unknown target) | ControlFlow.cs:12:9:12:87 | ...; | | ControlFlow.cs:10:9:10:43 | call to method | ControlFlow.cs:12:9:12:87 | ...; | | ControlFlow.cs:10:9:10:44 | ...; | ControlFlow.cs:10:9:10:13 | Expression | -| ControlFlow.cs:10:22:10:22 | access to local variable v | ControlFlow.cs:10:22:10:24 | Expression | -| ControlFlow.cs:10:22:10:24 | Expression | ControlFlow.cs:10:22:10:26 | Expression | -| ControlFlow.cs:10:22:10:26 | Expression | ControlFlow.cs:10:29:10:42 | "This is true" | +| ControlFlow.cs:10:22:10:22 | access to local variable v | ControlFlow.cs:10:22:10:24 | Call (unknown target) | +| ControlFlow.cs:10:22:10:22 | access to local variable v | ControlFlow.cs:10:22:10:24 | access to property (unknown) | +| ControlFlow.cs:10:22:10:24 | Call (unknown target) | ControlFlow.cs:10:22:10:26 | Call (unknown target) | +| ControlFlow.cs:10:22:10:24 | Call (unknown target) | ControlFlow.cs:10:22:10:26 | access to property (unknown) | +| ControlFlow.cs:10:22:10:24 | access to property (unknown) | ControlFlow.cs:10:22:10:26 | Call (unknown target) | +| ControlFlow.cs:10:22:10:24 | access to property (unknown) | ControlFlow.cs:10:22:10:26 | access to property (unknown) | +| ControlFlow.cs:10:22:10:26 | Call (unknown target) | ControlFlow.cs:10:29:10:42 | "This is true" | +| ControlFlow.cs:10:22:10:26 | access to property (unknown) | ControlFlow.cs:10:29:10:42 | "This is true" | | ControlFlow.cs:10:29:10:42 | "This is true" | ControlFlow.cs:10:9:10:43 | Call (unknown target) | | ControlFlow.cs:10:29:10:42 | "This is true" | ControlFlow.cs:10:9:10:43 | call to method | | ControlFlow.cs:12:9:12:86 | Call (unknown target) | ControlFlow.cs:12:37:12:47 | Expression | @@ -20,5 +25,7 @@ | ControlFlow.cs:12:51:12:62 | access to field Empty | ControlFlow.cs:12:37:12:62 | ... = ... | | ControlFlow.cs:12:65:12:75 | Expression | ControlFlow.cs:12:79:12:79 | access to local variable v | | ControlFlow.cs:12:65:12:84 | ... = ... | ControlFlow.cs:12:35:12:86 | { ..., ... } | -| ControlFlow.cs:12:79:12:79 | access to local variable v | ControlFlow.cs:12:79:12:84 | Expression | -| ControlFlow.cs:12:79:12:84 | Expression | ControlFlow.cs:12:65:12:84 | ... = ... | +| ControlFlow.cs:12:79:12:79 | access to local variable v | ControlFlow.cs:12:79:12:84 | Call (unknown target) | +| ControlFlow.cs:12:79:12:79 | access to local variable v | ControlFlow.cs:12:79:12:84 | access to property (unknown) | +| ControlFlow.cs:12:79:12:84 | Call (unknown target) | ControlFlow.cs:12:65:12:84 | ... = ... | +| ControlFlow.cs:12:79:12:84 | access to property (unknown) | ControlFlow.cs:12:65:12:84 | ... = ... | diff --git a/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected b/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected index 05ae24318c33..d57c3c86bbd9 100644 --- a/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected +++ b/csharp/ql/test/library-tests/standalone/errorrecovery/ErrorCalls.expected @@ -2,5 +2,5 @@ | errors.cs:43:21:43:28 | errors.cs:43:21:43:28 | object creation of type C1 | C1 | | errors.cs:44:13:44:19 | errors.cs:44:13:44:19 | call to method m1 | m1 | | errors.cs:45:13:45:19 | errors.cs:45:13:45:19 | call to method m2 | m2 | -| errors.cs:46:13:46:38 | errors.cs:46:13:46:38 | call to method | none | +| errors.cs:46:13:46:38 | errors.cs:46:13:46:38 | call to method WriteLine | WriteLine | | errors.cs:53:17:53:25 | errors.cs:53:17:53:25 | object creation of type C2 | none | diff --git a/csharp/ql/test/library-tests/standalone/regressions/ConstCase.expected b/csharp/ql/test/library-tests/standalone/regressions/ConstCase.expected index 47045b907f69..2c8616d347a7 100644 --- a/csharp/ql/test/library-tests/standalone/regressions/ConstCase.expected +++ b/csharp/ql/test/library-tests/standalone/regressions/ConstCase.expected @@ -1,2 +1,3 @@ -| regressions.cs:16:13:16:37 | case ...: | regressions.cs:16:18:16:36 | Expression | -| regressions.cs:18:13:18:37 | case ...: | regressions.cs:18:18:18:36 | Expression | +| regressions.cs:16:13:16:37 | case ...: | regressions.cs:16:18:16:36 | access to property (unknown) | +| regressions.cs:18:13:18:37 | case ...: | regressions.cs:18:18:18:36 | access to property (unknown) | +| regressions.cs:20:13:20:23 | case ...: | regressions.cs:20:18:20:22 | Int32 x | diff --git a/csharp/ql/test/library-tests/standalone/regressions/ConstCase.ql b/csharp/ql/test/library-tests/standalone/regressions/ConstCase.ql index 7ee7754e91e9..6c5d54518fec 100644 --- a/csharp/ql/test/library-tests/standalone/regressions/ConstCase.ql +++ b/csharp/ql/test/library-tests/standalone/regressions/ConstCase.ql @@ -1,7 +1,5 @@ import csharp from Case c, Expr e -where - e = c.getPattern().stripCasts() and - (e instanceof @unknown_expr or e instanceof ConstantPatternExpr) +where e = c.getPattern().stripCasts() select c, e