Merge pull request #209 from pjanotti/corelib.support

Add System.Private.CoreLib support

Author: tonerdo

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -66,6 +66,10 @@ private void InstrumentModule()
             {
                 resolver.AddSearchDirectory(Path.GetDirectoryName(_module));
                 var parameters = new ReaderParameters { ReadSymbols = true, AssemblyResolver = resolver };
+                if (Path.GetFileNameWithoutExtension(_module) == "System.Private.CoreLib")
+                {
+                    parameters.MetadataImporterProvider = new CoreLibMetadataImporterProvider();
+                }
 
                 using (var module = ModuleDefinition.ReadModule(stream, parameters))
                 {
@@ -108,7 +112,7 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)
                 foreach (FieldDefinition fieldDef in moduleTrackerTemplate.Fields)
                 {
                     var fieldClone = new FieldDefinition(fieldDef.Name, fieldDef.Attributes, fieldDef.FieldType);
-                    fieldClone.FieldType = module.ImportReference(fieldClone.FieldType);
+                    fieldClone.FieldType = module.ImportReference(fieldDef.FieldType);
 
                     _customTrackerTypeDef.Fields.Add(fieldClone);
 
@@ -170,7 +174,14 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module)
                     }
 
                     foreach (var handler in methodDef.Body.ExceptionHandlers)
+                    {
+                        if (handler.CatchType != null)
+                        {
+                            handler.CatchType = module.ImportReference(handler.CatchType);
+                        }
+                        
                         methodOnCustomType.Body.ExceptionHandlers.Add(handler);
+                    }
 
                     _customTrackerTypeDef.Methods.Add(methodOnCustomType);
                 }
@@ -406,5 +417,72 @@ private static Mono.Cecil.Cil.MethodBody GetMethodBody(MethodDefinition method)
                 return null;
             }
         }
+
+        /// <summary>
+        /// A custom importer created specifically to allow the instrumentation of System.Private.CoreLib by
+        /// removing the external references to netstandard that are generated when instrumenting a typical
+        /// assembly.
+        /// </summary>
+        private class CoreLibMetadataImporterProvider : IMetadataImporterProvider
+        {
+            public IMetadataImporter GetMetadataImporter(ModuleDefinition module)
+            {
+                return new CoreLibMetadataImporter(module);
+            }
+
+            private class CoreLibMetadataImporter : IMetadataImporter
+            {
+                private readonly ModuleDefinition module;
+                private readonly DefaultMetadataImporter defaultMetadataImporter;
+
+                public CoreLibMetadataImporter(ModuleDefinition module)
+                {
+                    this.module = module;
+                    this.defaultMetadataImporter = new DefaultMetadataImporter(module);
+                }
+
+                public AssemblyNameReference ImportReference(AssemblyNameReference reference)
+                {
+                    return this.defaultMetadataImporter.ImportReference(reference);
+                }
+
+                public TypeReference ImportReference(TypeReference type, IGenericParameterProvider context)
+                {
+                    var importedRef = this.defaultMetadataImporter.ImportReference(type, context);
+                    importedRef.GetElementType().Scope = module.TypeSystem.CoreLibrary;
+                    return importedRef;
+                }
+
+                public FieldReference ImportReference(FieldReference field, IGenericParameterProvider context)
+                {
+                    var importedRef = this.defaultMetadataImporter.ImportReference(field, context);
+                    importedRef.FieldType.GetElementType().Scope = module.TypeSystem.CoreLibrary;
+                    return importedRef;
+                }
+
+                public MethodReference ImportReference(MethodReference method, IGenericParameterProvider context)
+                {
+                    var importedRef = this.defaultMetadataImporter.ImportReference(method, context);
+                    importedRef.DeclaringType.GetElementType().Scope = module.TypeSystem.CoreLibrary;
+
+                    foreach (var parameter in importedRef.Parameters)
+                    {
+                        if (parameter.ParameterType.Scope == module.TypeSystem.CoreLibrary)
+                        {
+                            continue;
+                        }
+
+                        parameter.ParameterType.GetElementType().Scope = module.TypeSystem.CoreLibrary;
+                    }
+
+                    if (importedRef.ReturnType.Scope != module.TypeSystem.CoreLibrary)
+                    {
+                        importedRef.ReturnType.GetElementType().Scope = module.TypeSystem.CoreLibrary;
+                    }
+
+                    return importedRef;
+                }
+            }
+        }
     }
 }

src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs

@@ -20,26 +20,37 @@ public static class ModuleTrackerTemplate
         public static string HitsFilePath;
         public static int[] HitsArray;
 
+        // Special case when instrumenting CoreLib, the thread static below prevents infinite loop in CoreLib
+        // while allowing the tracker template to call any of its types and functions.
+        [ThreadStatic]
+        private static bool t_isTracking;
+
         [ThreadStatic]
         private static int[] t_threadHits;
 
         private static List<int[]> _threads;
 
         static ModuleTrackerTemplate()
         {
+            t_isTracking = true;
             _threads = new List<int[]>(2 * Environment.ProcessorCount);
 
             AppDomain.CurrentDomain.ProcessExit += new EventHandler(UnloadModule);
             AppDomain.CurrentDomain.DomainUnload += new EventHandler(UnloadModule);
+            t_isTracking = false;
             // At the end of the instrumentation of a module, the instrumenter needs to add code here
             // to initialize the static fields according to the values derived from the instrumentation of
             // the module.
         }
 
         public static void RecordHit(int hitLocationIndex)
         {
+            if (t_isTracking)
+                return;
+
             if (t_threadHits == null)
             {
+                t_isTracking = true;
                 lock (_threads)
                 {
                     if (t_threadHits == null)
@@ -48,13 +59,16 @@ public static void RecordHit(int hitLocationIndex)
                         _threads.Add(t_threadHits);
                     }
                 }
+                t_isTracking = false;
             }
 
             ++t_threadHits[hitLocationIndex];
         }
 
         public static void UnloadModule(object sender, EventArgs e)
         {
+            t_isTracking = true;
+
             // Update the global hits array from data from all the threads
             lock (_threads)
             {
@@ -76,10 +90,10 @@ public static void UnloadModule(object sender, EventArgs e)
                 if (!createdNew)
                     mutex.WaitOne();
 
-                if (!File.Exists(HitsFilePath))
+                bool failedToCreateNewHitsFile = false;
+                try
                 {
-                    // File not created yet, just write it
-                    using (var fs = new FileStream(HitsFilePath, FileMode.Create))
+                    using (var fs = new FileStream(HitsFilePath, FileMode.CreateNew))
                     using (var bw = new BinaryWriter(fs))
                     {
                         bw.Write(HitsArray.Length);
@@ -89,18 +103,23 @@ public static void UnloadModule(object sender, EventArgs e)
                         }
                     }
                 }
-                else
+                catch
+                {
+                    failedToCreateNewHitsFile = true;
+                }
+
+                if (failedToCreateNewHitsFile)
                 {
                     // Update the number of hits by adding value on disk with the ones on memory.
                     // This path should be triggered only in the case of multiple AppDomain unloads.
-                    using (var fs = File.Open(HitsFilePath, FileMode.Open))
+                    using (var fs = new FileStream(HitsFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None))
                     using (var br = new BinaryReader(fs))
                     using (var bw = new BinaryWriter(fs))
                     {
                         int hitsLength = br.ReadInt32();
                         if (hitsLength != HitsArray.Length)
                         {
-                            throw new InvalidDataException(
+                            throw new InvalidOperationException(
                                 $"{HitsFilePath} has {hitsLength} entries but on memory {nameof(HitsArray)} has {HitsArray.Length}");
                         }
 
@@ -116,6 +135,10 @@ public static void UnloadModule(object sender, EventArgs e)
                 // Prevent any double counting scenario, i.e.: UnloadModule called twice (not sure if this can happen in practice ...)
                 // Only an issue if DomainUnload and ProcessExit can both happens, perhaps can be removed...
                 Array.Clear(HitsArray, 0, HitsArray.Length);
+
+                // On purpose this is not under a try-finally: it is better to have an exception if there was any error writing the hits file
+                // this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
+                mutex.ReleaseMutex();
             }
         }
     }

test/coverlet.core.performancetest/PerformanceTest.cs

@@ -14,8 +14,7 @@ namespace coverlet.core.performancetest
     /// </summary>
     public class PerformanceTest
     {
-        [Theory(/*Skip = "Only enabled when explicitly testing performance."*/)]
-        // [InlineData(150)]
+        [Theory]
         [InlineData(20_000)]
         public void TestPerformance(int iterations)
         {

test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs

@@ -9,6 +9,30 @@ namespace Coverlet.Core.Instrumentation.Tests
 {
     public class InstrumenterTests
     {
+        [Fact(Skip = "To be used only validating System.Private.CoreLib instrumentation")]
+        public void TestCoreLibInstrumentation()
+        {
+            // Attention: to run this test adjust the paths and copy the IL only version of corelib
+            const string OriginalFilesDir = @"c:\s\tmp\Coverlet-CoreLib\Original\";
+            const string TestFilesDir = @"c:\s\tmp\Coverlet-CoreLib\Test\";
+
+            Directory.CreateDirectory(TestFilesDir);
+
+            string[] files = new[]
+            {
+                "System.Private.CoreLib.dll",
+                "System.Private.CoreLib.pdb"
+            };
+
+            foreach (var file in files)
+                File.Copy(Path.Combine(OriginalFilesDir, file), Path.Combine(TestFilesDir, file), overwrite: true);
+
+            Instrumenter instrumenter = new Instrumenter(Path.Combine(TestFilesDir, files[0]), "_coverlet_instrumented", Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>());
+            Assert.True(instrumenter.CanInstrument());
+            var result = instrumenter.Instrument();
+            Assert.NotNull(result);
+        }
+
         [Fact]
         public void TestInstrument()
         {
@@ -22,6 +46,19 @@ public void TestInstrument()
             instrumenterTest.Directory.Delete(true);
         }
 
+        [Fact]
+        public void TestInstrumentCoreLib()
+        {
+            var instrumenterTest = CreateInstrumentor(fakeCoreLibModule: true);
+
+            var result = instrumenterTest.Instrumenter.Instrument();
+
+            Assert.Equal(Path.GetFileNameWithoutExtension(instrumenterTest.Module), result.Module);
+            Assert.Equal(instrumenterTest.Module, result.ModulePath);
+
+            instrumenterTest.Directory.Delete(true);
+        }
+
         [Theory]
         [InlineData(typeof(ClassExcludedByCodeAnalysisCodeCoverageAttr))]
         [InlineData(typeof(ClassExcludedByCoverletCodeCoverageAttr))]
@@ -39,18 +76,30 @@ public void TestInstrument_ClassesWithExcludeAttributeAreExcluded(Type excludedT
             instrumenterTest.Directory.Delete(true);
         }
 
-        private InstrumenterTest CreateInstrumentor()
+        private InstrumenterTest CreateInstrumentor(bool fakeCoreLibModule = false)
         {
             string module = GetType().Assembly.Location;
             string pdb = Path.Combine(Path.GetDirectoryName(module), Path.GetFileNameWithoutExtension(module) + ".pdb");
             string identifier = Guid.NewGuid().ToString();
 
             DirectoryInfo directory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), identifier));
 
-            File.Copy(module, Path.Combine(directory.FullName, Path.GetFileName(module)), true);
-            File.Copy(pdb, Path.Combine(directory.FullName, Path.GetFileName(pdb)), true);
+            string destModule, destPdb;
+            if (fakeCoreLibModule)
+            {
+                destModule = "System.Private.CoreLib.dll";
+                destPdb = "System.Private.CoreLib.pdb";
+            }
+            else
+            {
+                destModule = Path.GetFileName(module);
+                destPdb = Path.GetFileName(pdb);
+            }
+
+            File.Copy(module, Path.Combine(directory.FullName, destModule), true);
+            File.Copy(pdb, Path.Combine(directory.FullName, destPdb), true);
 
-            module = Path.Combine(directory.FullName, Path.GetFileName(module));
+            module = Path.Combine(directory.FullName, destModule);
             Instrumenter instrumenter = new Instrumenter(module, identifier, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>());
             return new InstrumenterTest
             {

test/coverlet.core.tests/Instrumentation/ModuleTrackerTemplateTests.cs

@@ -49,7 +49,7 @@ public void HitsFileWithDifferentNumberOfEntriesCausesExceptionOnUnload()
         {
             WriteHitsFile(new[] { 1, 2, 3 });
             ModuleTrackerTemplate.HitsArray = new[] { 1 };
-            Assert.Throws<InvalidDataException>(() => ModuleTrackerTemplate.UnloadModule(null, null));
+            Assert.Throws<InvalidOperationException>(() => ModuleTrackerTemplate.UnloadModule(null, null));
         }
 
         [Fact]