From 7bdde4efe28cc3d7b86b1b1b460b903b24477762 Mon Sep 17 00:00:00 2001 From: Alex Fedotov Date: Thu, 6 Aug 2015 17:37:04 -0700 Subject: [PATCH 1/2] Properly escaping type names when serializing metadata. --- System.Compiler/Writer.cs | 73 ++++++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/System.Compiler/Writer.cs b/System.Compiler/Writer.cs index 02cefbfe..d18a6f55 100644 --- a/System.Compiler/Writer.cs +++ b/System.Compiler/Writer.cs @@ -4678,16 +4678,27 @@ string GetSerializedTypeName(TypeNode/*!*/ type, ref bool isAssemblyQualified) { sb.Append('&'); goto done; } + if (type.Template == null) - sb.Append(type.FullName); - else{ - sb.Append(type.Template.FullName); + { + AppendEscapedTypeName(sb, type); + } + else + { + AppendEscapedTypeName(sb, type.Template); + sb.Append('['); - for (int i = 0, n = type.ConsolidatedTemplateArguments == null ? 0 : type.ConsolidatedTemplateArguments.Count; i < n; i++) { - //^ assert type.ConsolidatedTemplateArguments != null; - bool isAssemQual = true; - this.AppendSerializedTypeName(sb, type.ConsolidatedTemplateArguments[i], ref isAssemQual); - if (i < n-1) sb.Append(','); + if (type.ConsolidatedTemplateArguments != null) + { + for (int i = 0; i < type.ConsolidatedTemplateArguments.Count; i++) + { + if (i > 0) + sb.Append(','); + + //^ assert type.ConsolidatedTemplateArguments != null; + bool isAssemQual = true; + this.AppendSerializedTypeName(sb, type.ConsolidatedTemplateArguments[i], ref isAssemQual); + } } sb.Append(']'); } @@ -4696,6 +4707,52 @@ string GetSerializedTypeName(TypeNode/*!*/ type, ref bool isAssemblyQualified) { this.AppendAssemblyQualifierIfNecessary(sb, type, out isAssemblyQualified); return sb.ToString(); } + + /// + /// Appends the escaped full name of the type to a string builder. + /// + /// String builder to append to. + /// Type whose full name to append. + void AppendEscapedTypeName(StringBuilder sb, TypeNode type) + { + // NOTE: we have to carefully deal with types defined within other types to avoid + // escaping the '+' character when it is used to separate inner types from their + // declaring type. + if (type.DeclaringType != null) + { + // if this type is defined within another type, append the declaring type name first + // followed by the '+' separator + AppendEscapedTypeName(sb, type.DeclaringType); + sb.Append('+'); + } + else if (type.Namespace != null) + { + // append the namespace name followed by a dot + sb.Append(type.Namespace.Name); + sb.Append('.'); + } + + // deal with the actual type name + foreach (char c in type.Name.Name) + { + if (IsTypeNameReservedChar(c)) + sb.Append('\\'); + sb.Append(c); + } + } + + /// + /// Determines if the provided character has a special meaning in a fully qualified type name + /// and therefore has to be escaped if used as part of a type name identifier. + /// + /// Character to test. + /// True if the character has to be escaped or false - otherwise. + static bool IsTypeNameReservedChar(char ch) + { + return ch == ',' || ch == '[' || ch == ']' || ch == '&' || + ch == '*' || ch == '+' || ch == '\\'; + } + void AppendAssemblyQualifierIfNecessary(StringBuilder/*!*/ sb, TypeNode type, out bool isAssemQualified) { isAssemQualified = false; if (type == null) return; From 809fbddef2c98e466691e849ab61eb2563daaad2 Mon Sep 17 00:00:00 2001 From: Alex Fedotov Date: Fri, 7 Aug 2015 11:31:40 -0700 Subject: [PATCH 2/2] Updated unit test to verify #186. --- Foxtrot/Tests/FoxtrotTests10.csproj | 8 +++++++ Foxtrot/Tests/Options.cs | 6 +++++ Foxtrot/Tests/RewriterTests.cs | 4 +++- Foxtrot/Tests/TestDriver.cs | 35 +++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/Foxtrot/Tests/FoxtrotTests10.csproj b/Foxtrot/Tests/FoxtrotTests10.csproj index 82c86f8d..b17ce3ef 100644 --- a/Foxtrot/Tests/FoxtrotTests10.csproj +++ b/Foxtrot/Tests/FoxtrotTests10.csproj @@ -142,6 +142,14 @@ {9D404D0C-0921-4ADD-BC22-A14AAE2CA7E2} CRASanitizer + + {d7e16b38-3893-4eef-847f-a3be807e9546} + System.CompilerCC + + + {f7364559-90f5-480b-b975-c95a04e0daf6} + Foxtrot.Extractor + {8602713E-68E4-4A8F-BB6C-9AEA279E494F} AssemblyWithContracts diff --git a/Foxtrot/Tests/Options.cs b/Foxtrot/Tests/Options.cs index 4ce1fb2c..f868ad01 100644 --- a/Foxtrot/Tests/Options.cs +++ b/Foxtrot/Tests/Options.cs @@ -47,6 +47,12 @@ public string ReferencesFramework public bool UseExe = true; + /// + /// If set to true, verification will include reading contracts back from the rewritten + /// assembly in addition to running peverify. + /// + public bool DeepVerify = false; + private int instance; private TestContext TestContext; diff --git a/Foxtrot/Tests/RewriterTests.cs b/Foxtrot/Tests/RewriterTests.cs index d6cd406d..0f2cc777 100644 --- a/Foxtrot/Tests/RewriterTests.cs +++ b/Foxtrot/Tests/RewriterTests.cs @@ -114,7 +114,8 @@ public void Roslyn_CompatibilityCheckInReleaseMode() } /// - /// Unit test for #47 - "Could not resolve type reference" for some iterator methods in VS2015 + /// Unit test for #47 - "Could not resolve type reference" for some iterator methods in VS2015 and + /// #186 - ccrewrite produces an incorrect type name in IteratorStateMachineAttribute with some generic types /// [DeploymentItem("Foxtrot\\Tests\\TestInputs.xml"), DataSource("Microsoft.VisualStudio.TestTools.DataSource.XML", "|DataDirectory|\\TestInputs.xml", "IteratorWithComplexGeneric", DataAccessMethod.Sequential)] [TestMethod] @@ -125,6 +126,7 @@ public void Roslyn_IteratorBlockWithComplexGeneric() // Bug with metadata reader could be reproduced only with a new mscorlib and new System.dll. options.BuildFramework = @".NetFramework\v4.6"; options.ReferencesFramework = @".NetFramework\v4.6"; + options.DeepVerify = true; TestDriver.BuildRewriteRun(options); } diff --git a/Foxtrot/Tests/TestDriver.cs b/Foxtrot/Tests/TestDriver.cs index 18ca15c5..08de455b 100644 --- a/Foxtrot/Tests/TestDriver.cs +++ b/Foxtrot/Tests/TestDriver.cs @@ -18,6 +18,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using System.Text; using System.Collections.Generic; +using Microsoft.Contracts.Foxtrot; namespace Tests { @@ -112,6 +113,37 @@ private static int PEVerify(string assemblyFile, Options options) return exitCode; } + /// + /// Attempts to extract contracts from the specified assembly file. Used to verify that + /// Foxtrot can still extract contracts from a rewritten assembly. + /// + private static void ExtractContracts(string assemblyFile, Options options) + { + // use assembly resolver from Foxtrot.Extractor + var resolver = new AssemblyResolver( + resolvedPaths: new string[0], + libpaths: options.LibPaths, + usePDB: false, + preserveShortBranches: true, + trace: false, + postLoad: (r, assemblyNode) => { + ContractNodes contractNodes = null; + Extractor.ExtractContracts( + assemblyNode, null, null, null, null, out contractNodes, + e => Assert.Fail(e.ToString()), false); + }); + + var assembly = resolver.ProbeForAssembly( + assemblyName: Path.GetFileNameWithoutExtension(assemblyFile), + referencingModuleDirectory: Path.GetDirectoryName(assemblyFile), + exts: new string[] { Path.GetExtension(assemblyFile) }); + + // the assembly must be resolved and have no metadata import errors + Assert.IsNotNull(assembly); + Assert.IsTrue(assembly.MetadataImportErrors == null || assembly.MetadataImportErrors.Count == 0, + "Parsing back the rewritten assembly produced metadata import errors"); + } + internal static string RewriteAndVerify(string sourceDir, string binary, Options options) { if (!Path.IsPathRooted(sourceDir)) { sourceDir = options.MakeAbsolute(sourceDir); } @@ -120,6 +152,9 @@ internal static string RewriteAndVerify(string sourceDir, string binary, Options if (target != null) { PEVerify(target, options); + + if (options.DeepVerify) + ExtractContracts(target, options); } return target; }