From 371d732c0a41a07acaaa46317939f56f2ccdd937 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 29 Mar 2020 21:18:23 +0200 Subject: [PATCH] Fix #1881: Not properly reusing names from PDB#2 --- .../Helpers/Tester.cs | 21 ++++++++--- .../ICSharpCode.Decompiler.Tests.csproj | 4 +++ .../PdbGenerationTestRunner.cs | 2 +- .../PrettyTestRunner.cs | 2 +- .../TestCases/Pretty/VariableNaming.cs | 35 ++++++++++++++++++- .../IL/Transforms/AssignVariableNames.cs | 35 +++++++++++++------ 6 files changed, 80 insertions(+), 19 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs index 85e2cc4a1..0dc01b9b4 100644 --- a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs +++ b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs @@ -56,6 +56,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers UseMcs = 0x20, ReferenceVisualBasic = 0x40, ReferenceCore = 0x80, + GeneratePdb = 0x100, } [Flags] @@ -272,7 +273,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers preprocessorSymbols: preprocessorSymbols.ToArray(), languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp8 ); - var syntaxTrees = sourceFileNames.Select(f => SyntaxFactory.ParseSyntaxTree(File.ReadAllText(f), parseOptions, path: f)); + var syntaxTrees = sourceFileNames.Select(f => SyntaxFactory.ParseSyntaxTree(File.ReadAllText(f), parseOptions, path: f, encoding: Encoding.UTF8)); if (flags.HasFlag(CompilerOptions.ReferenceCore)) { syntaxTrees = syntaxTrees.Concat(new[] { SyntaxFactory.ParseSyntaxTree(targetFrameworkAttributeSnippet) }); } @@ -296,7 +297,10 @@ namespace ICSharpCode.Decompiler.Tests.Helpers )); CompilerResults results = new CompilerResults(new TempFileCollection()); results.PathToAssembly = outputFileName ?? Path.GetTempFileName(); - var emitResult = compilation.Emit(results.PathToAssembly); + string pdbName = null; + if (flags.HasFlag(CompilerOptions.GeneratePdb)) + pdbName = Path.ChangeExtension(outputFileName, ".pdb"); + var emitResult = compilation.Emit(results.PathToAssembly, pdbName); if (!emitResult.Success) { StringBuilder b = new StringBuilder("Compiler error:"); foreach (var diag in emitResult.Diagnostics) { @@ -360,7 +364,12 @@ namespace ICSharpCode.Decompiler.Tests.Helpers CompilerParameters options = new CompilerParameters(); options.GenerateExecutable = !flags.HasFlag(CompilerOptions.Library); options.CompilerOptions = "/unsafe /o" + (flags.HasFlag(CompilerOptions.Optimize) ? "+" : "-"); - options.CompilerOptions += (flags.HasFlag(CompilerOptions.UseDebug) ? " /debug" : ""); + string debugOption = " /debug"; + if (flags.HasFlag(CompilerOptions.GeneratePdb)) { + debugOption += ":full"; + options.IncludeDebugInformation = true; + } + options.CompilerOptions += (flags.HasFlag(CompilerOptions.UseDebug) ? debugOption : ""); options.CompilerOptions += (flags.HasFlag(CompilerOptions.Force32Bit) ? " /platform:anycpu32bitpreferred" : ""); if (preprocessorSymbols.Count > 0) { options.CompilerOptions += " /d:" + string.Join(";", preprocessorSymbols); @@ -368,7 +377,6 @@ namespace ICSharpCode.Decompiler.Tests.Helpers if (outputFileName != null) { options.OutputAssembly = outputFileName; } - options.ReferencedAssemblies.Add("System.dll"); options.ReferencedAssemblies.Add("System.Core.dll"); options.ReferencedAssemblies.Add("System.Xml.dll"); @@ -397,7 +405,7 @@ namespace ICSharpCode.Decompiler.Tests.Helpers } } - public static void CompileCSharpWithPdb(string assemblyName, Dictionary sourceFiles, PdbToXmlOptions options) + public static void CompileCSharpWithPdb(string assemblyName, Dictionary sourceFiles) { var parseOptions = new CSharpParseOptions(languageVersion: Microsoft.CodeAnalysis.CSharp.LanguageVersion.Latest); @@ -484,6 +492,9 @@ namespace ICSharpCode.Decompiler.Tests.Helpers decompiler.AstTransforms.Insert(0, new RemoveCompilerAttribute()); decompiler.AstTransforms.Insert(0, new RemoveNamespaceMy()); decompiler.AstTransforms.Add(new EscapeInvalidIdentifiers()); + var pdbFileName = Path.ChangeExtension(assemblyFileName, ".pdb"); + if (File.Exists(pdbFileName)) + decompiler.DebugInfoProvider = PdbProvider.DebugInfoUtils.FromFile(module, pdbFileName); var syntaxTree = decompiler.DecompileWholeModuleAsSingleFile(sortTypes: true); StringWriter output = new StringWriter(); diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 1f9c4232c..2dea3c340 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -49,6 +49,7 @@ + @@ -80,6 +81,9 @@ + + + diff --git a/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs b/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs index 78de11172..6077d6c15 100644 --- a/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs @@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.Tests string xmlContent = File.ReadAllText(xmlFile); XDocument document = XDocument.Parse(xmlContent); var files = document.Descendants("file").ToDictionary(f => f.Attribute("name").Value, f => f.Value); - Tester.CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), files, options); + Tester.CompileCSharpWithPdb(Path.Combine(TestCasePath, testName + ".expected"), files); string peFileName = Path.Combine(TestCasePath, testName + ".expected.dll"); string pdbFileName = Path.Combine(TestCasePath, testName + ".expected.pdb"); diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index 8af4d4a13..66456641c 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -284,7 +284,7 @@ namespace ICSharpCode.Decompiler.Tests [Test] public void VariableNaming([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions) { - RunForLibrary(cscOptions: cscOptions); + RunForLibrary(cscOptions: cscOptions | CompilerOptions.GeneratePdb); } [Test] diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNaming.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNaming.cs index 169299791..80b453a73 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNaming.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/VariableNaming.cs @@ -1,7 +1,17 @@ -namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +#if !OPT +using System; +#endif + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { internal class VariableNaming { + private enum MyEnum + { + VALUE1 = 1, + VALUE2 + } + private class C { public string Name; @@ -25,5 +35,28 @@ string text2 = c.Text; #endif } + +#if !OPT + private void Issue1841() + { + C gen1 = new C(); + C gen2 = new C(); + C gen3 = new C(); + C gen4 = new C(); + } + + private void Issue1881() + { + MyEnum enumLocal1 = MyEnum.VALUE1; + MyEnum enumLocal2 = (MyEnum)0; + enumLocal2 = MyEnum.VALUE1; + object enumLocal3 = MyEnum.VALUE2; + object enumLocal4 = new object(); + enumLocal4 = MyEnum.VALUE2; + ValueType enumLocal5 = MyEnum.VALUE1; + ValueType enumLocal6 = (MyEnum)0; + enumLocal6 = MyEnum.VALUE2; + } +#endif } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs b/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs index e5f28ff72..02b25933c 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/AssignVariableNames.cs @@ -20,12 +20,8 @@ using System; using System.Collections.Generic; using System.Linq; using System.Reflection; -using System.Text; -using System.Threading.Tasks; using Humanizer; using ICSharpCode.Decompiler.CSharp.OutputVisitor; -using ICSharpCode.Decompiler.IL; -using ICSharpCode.Decompiler.Semantics; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.Decompiler.TypeSystem.Implementation; using ICSharpCode.Decompiler.Util; @@ -141,6 +137,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // remove unused variables before assigning names function.Variables.RemoveDead(); int numDisplayClassLocals = 0; + Dictionary assignedLocalSignatureIndices = new Dictionary(); foreach (var v in function.Variables.OrderBy(v => v.Name)) { switch (v.Kind) { case VariableKind.Parameter: // ignore @@ -151,16 +148,32 @@ namespace ICSharpCode.Decompiler.IL.Transforms case VariableKind.DisplayClassLocal: v.Name = "CS$<>8__locals" + (numDisplayClassLocals++); break; - default: - if (v.HasGeneratedName || !IsValidName(v.Name) || ConflictWithLocal(v)) { - // don't use the name from the debug symbols if it looks like a generated name - v.Name = null; + case VariableKind.Local when v.Index != null: + if (assignedLocalSignatureIndices.TryGetValue(v.Index.Value, out string name)) { + // make sure all local ILVariables that refer to the same slot in the locals signature + // are assigned the same name. + v.Name = name; } else { - // use the name from the debug symbols - // (but ensure we don't use the same name for two variables) - v.Name = GetAlternativeName(v.Name); + AssignName(); + // Remember the newly assigned name: + assignedLocalSignatureIndices.Add(v.Index.Value, v.Name); } break; + default: + AssignName(); + break; + } + + void AssignName() + { + if (v.HasGeneratedName || !IsValidName(v.Name) || ConflictWithLocal(v)) { + // don't use the name from the debug symbols if it looks like a generated name + v.Name = null; + } else { + // use the name from the debug symbols + // (but ensure we don't use the same name for two variables) + v.Name = GetAlternativeName(v.Name); + } } } foreach (var localFunction in function.LocalFunctions) {