From c976a2e9d2aac5d6583b267af34fa651a7bce70d Mon Sep 17 00:00:00 2001 From: kohanis Date: Thu, 26 Dec 2024 22:45:39 +0300 Subject: [PATCH 1/5] CecilEmitter.Dump: Synchronizing dumper with monomod changes --- Harmony/Internal/Util/CecilEmitter.cs | 39 ++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/Harmony/Internal/Util/CecilEmitter.cs b/Harmony/Internal/Util/CecilEmitter.cs index e328752f..997b7b23 100644 --- a/Harmony/Internal/Util/CecilEmitter.cs +++ b/Harmony/Internal/Util/CecilEmitter.cs @@ -19,7 +19,7 @@ namespace HarmonyLib.Internal.Util; /// /// Basic safe DLL emitter for dynamically generated s. /// -/// Based on https://github.com/MonoMod/MonoMod.Common/blob/master/Utils/DMDGenerators/DMDCecilGenerator.cs +/// Based on https://github.com/MonoMod/MonoMod/blob/reorganize/src/MonoMod.Utils/DMDGenerators/DMDCecilGenerator.cs internal static class CecilEmitter { private static readonly ConstructorInfo UnverifiableCodeAttributeConstructor = @@ -52,7 +52,21 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth var isVolatile = new TypeReference("System.Runtime.CompilerServices", "IsVolatile", module, module.TypeSystem.CoreLibrary); - Relinker relinker = (mtp, _) => mtp == md ? clone : module.ImportReference(mtp); +#pragma warning disable IDE0039 // Use local function + Relinker relinker = (mtp, ctx) => + { + if (mtp == md) + return clone!; + if (mtp is MethodReference mr) + { + if (mr.FullName == md.FullName + && mr.DeclaringType.FullName == md.DeclaringType.FullName + && mr.DeclaringType.Scope.Name == md.DeclaringType.Scope.Name) + return clone!; + } + return module.ImportReference(mtp); + }; +#pragma warning restore IDE0039 // Use local function clone = new MethodDefinition(original?.Name ?? "_" + md.Name.Replace(".", "_"), md.Attributes, module.TypeSystem.Void) @@ -61,7 +75,8 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth Attributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Static, ImplAttributes = MethodImplAttributes.IL | MethodImplAttributes.Managed, DeclaringType = td, - HasThis = false + HasThis = false, + NoInlining = true }; td.Methods.Add(clone); @@ -88,10 +103,20 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth _ => operand }; - if (instr.Previous?.OpCode == OpCodes.Volatile && - operand is FieldReference fref && - (fref.FieldType as RequiredModifierType)?.ModifierType != isVolatile) - fref.FieldType = new RequiredModifierType(isVolatile, fref.FieldType); + // System.Reflection doesn't contain any volatility info. + // System.Reflection.Emit presumably does something similar to this. + // Mono.Cecil thus isn't aware of the volatility as part of the imported field reference. + // The modifier is still necessary though. + // This is done here instead of the copier as Harmony and other users can't track modreqs + + // This isn't actually a valid transformation though. A ldfld or stfld can have the volatile + // prefix, without having modreq(IsVolatile) on the field. Adding the modreq() causes the runtime + // to not be able to find the field. + /*if (instr.Previous?.OpCode == OpCodes.Volatile && + operand is FieldReference fref && + (fref.FieldType as RequiredModifierType)?.ModifierType != tr_IsVolatile) { + fref.FieldType = new RequiredModifierType(tr_IsVolatile, fref.FieldType); + }*/ instr.Operand = operand; } From c7c196d6ca60191a1dd1a302a86c082a2158598d Mon Sep 17 00:00:00 2001 From: kohanis Date: Thu, 26 Dec 2024 22:48:40 +0300 Subject: [PATCH 2/5] CecilEmitter.Dump: Tidying up --- Harmony/Internal/Util/CecilEmitter.cs | 37 +++++++++++++-------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/Harmony/Internal/Util/CecilEmitter.cs b/Harmony/Internal/Util/CecilEmitter.cs index 997b7b23..599d2ac6 100644 --- a/Harmony/Internal/Util/CecilEmitter.cs +++ b/Harmony/Internal/Util/CecilEmitter.cs @@ -47,13 +47,20 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth module.Types.Add(td); - MethodDefinition clone = null; - - var isVolatile = new TypeReference("System.Runtime.CompilerServices", "IsVolatile", module, - module.TypeSystem.CoreLibrary); + var clone = + new MethodDefinition(original?.Name ?? "_" + md.Name.Replace(".", "_"), md.Attributes, module.TypeSystem.Void) + { + MethodReturnType = md.MethodReturnType, + Attributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Static, + ImplAttributes = MethodImplAttributes.IL | MethodImplAttributes.Managed, + DeclaringType = td, + HasThis = false, + NoInlining = true + }; + td.Methods.Add(clone); #pragma warning disable IDE0039 // Use local function - Relinker relinker = (mtp, ctx) => + Relinker relinker = (mtp, _) => { if (mtp == md) return clone!; @@ -68,30 +75,22 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth }; #pragma warning restore IDE0039 // Use local function - clone = - new MethodDefinition(original?.Name ?? "_" + md.Name.Replace(".", "_"), md.Attributes, module.TypeSystem.Void) - { - MethodReturnType = md.MethodReturnType, - Attributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Static, - ImplAttributes = MethodImplAttributes.IL | MethodImplAttributes.Managed, - DeclaringType = td, - HasThis = false, - NoInlining = true - }; - td.Methods.Add(clone); - foreach (var param in md.Parameters) clone.Parameters.Add(param.Clone().Relink(relinker, clone)); clone.ReturnType = md.ReturnType.Relink(relinker, clone); var body = clone.Body = md.Body.Clone(clone); - foreach (var variable in clone.Body.Variables) + foreach (var variable in body.Variables) variable.VariableType = variable.VariableType.Relink(relinker, clone); - foreach (var handler in clone.Body.ExceptionHandlers.Where(handler => handler.CatchType != null)) + foreach (var handler in body.ExceptionHandlers.Where(handler => handler.CatchType != null)) handler.CatchType = handler.CatchType.Relink(relinker, clone); + /* see below + var isVolatile = new TypeReference("System.Runtime.CompilerServices", "IsVolatile", module, + module.TypeSystem.CoreLibrary); + */ foreach (var instr in body.Instructions) { var operand = instr.Operand; From e89e21559e3020d72278d898716baf907005e41a Mon Sep 17 00:00:00 2001 From: kohanis Date: Thu, 26 Dec 2024 22:51:34 +0300 Subject: [PATCH 3/5] CecilEmitter.Dump: Adding argument check --- Harmony/Internal/Util/CecilEmitter.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Harmony/Internal/Util/CecilEmitter.cs b/Harmony/Internal/Util/CecilEmitter.cs index 599d2ac6..dc8495fa 100644 --- a/Harmony/Internal/Util/CecilEmitter.cs +++ b/Harmony/Internal/Util/CecilEmitter.cs @@ -27,6 +27,9 @@ internal static class CecilEmitter public static void Dump(MethodDefinition md, IEnumerable dumpPaths, MethodBase original = null) { + if(md.Body is null) + throw new ArgumentException($"Body of MethodDefinition '{md.Name}' to dump is null"); + var name = $"HarmonyDump.{SanitizeTypeName(md.GetID(withType: false, simple: true))}.{Guid.NewGuid().GetHashCode():X8}"; var originalName = (original?.Name ?? md.Name).Replace('.', '_'); using var module = ModuleDefinition.CreateModule(name, From c48023cf3eb0de3b2ab58687f4a43d4f14ac1c85 Mon Sep 17 00:00:00 2001 From: kohanis Date: Thu, 26 Dec 2024 22:52:52 +0300 Subject: [PATCH 4/5] CecilEmitter.Dump: Fixing dumping ILLabels --- Harmony/Internal/Util/CecilEmitter.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Harmony/Internal/Util/CecilEmitter.cs b/Harmony/Internal/Util/CecilEmitter.cs index dc8495fa..8c79e613 100644 --- a/Harmony/Internal/Util/CecilEmitter.cs +++ b/Harmony/Internal/Util/CecilEmitter.cs @@ -1,10 +1,12 @@ using HarmonyLib.Tools; using Mono.Cecil; using Mono.Cecil.Cil; +using Mono.Collections.Generic; using MonoMod.Cil; using MonoMod.Utils; using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Reflection; @@ -100,7 +102,7 @@ public static void Dump(MethodDefinition md, IEnumerable dumpPaths, Meth operand = operand switch { ParameterDefinition param => clone.Parameters[param.Index], - ILLabel label => label.Target, + ILLabel label => LabelFix(label, body.Instructions, md.Body.Instructions, originalName), IMetadataTokenProvider mtp => mtp.Relink(relinker, clone), _ => operand }; @@ -156,4 +158,14 @@ private static string SanitizeTypeName(string typeName) .Replace("<", "{") .Replace(">", "}"); } + + private static Instruction LabelFix(ILLabel label, Collection clone, Collection original, string originalName) + { + Debug.Assert(clone.Count == original.Count); + var target = label.Target; + var idx = original.IndexOf(target); + if (idx == -1) + throw new NullReferenceException($"ILLabel from dump copy of '{originalName}' cannot be relinked. It points to '{target}', which was not found in the original body"); + return clone[idx]; + } } From fb1248b98818f1a7ab443080e37e36b637f1e9c7 Mon Sep 17 00:00:00 2001 From: kohanis Date: Fri, 27 Dec 2024 00:13:00 +0300 Subject: [PATCH 5/5] CecilEmitter.Dump: Adding test for ILLabel dumping --- Harmony/Internal/Util/CecilEmitter.cs | 37 ++++++++++++++++----------- HarmonyTests/Extras/Dump.cs | 36 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 HarmonyTests/Extras/Dump.cs diff --git a/Harmony/Internal/Util/CecilEmitter.cs b/Harmony/Internal/Util/CecilEmitter.cs index 8c79e613..594766cb 100644 --- a/Harmony/Internal/Util/CecilEmitter.cs +++ b/Harmony/Internal/Util/CecilEmitter.cs @@ -28,13 +28,33 @@ internal static class CecilEmitter typeof(UnverifiableCodeAttribute).GetConstructor(Type.EmptyTypes); public static void Dump(MethodDefinition md, IEnumerable dumpPaths, MethodBase original = null) + { + using var module = DumpImpl(md, original); + + foreach (var settingsDumpPath in dumpPaths) + { + var fullPath = Path.GetFullPath(settingsDumpPath); + try + { + Directory.CreateDirectory(fullPath); + using var stream = File.OpenWrite(Path.Combine(fullPath, $"{module.Name}.dll")); + module.Write(stream); + } + catch (Exception e) + { + Logger.Log(Logger.LogChannel.Error, () => $"Failed to dump {md.GetID(simple: true)} to {fullPath}: {e}"); + } + } + } + + internal static ModuleDefinition DumpImpl(MethodDefinition md, MethodBase original = null) { if(md.Body is null) throw new ArgumentException($"Body of MethodDefinition '{md.Name}' to dump is null"); var name = $"HarmonyDump.{SanitizeTypeName(md.GetID(withType: false, simple: true))}.{Guid.NewGuid().GetHashCode():X8}"; var originalName = (original?.Name ?? md.Name).Replace('.', '_'); - using var module = ModuleDefinition.CreateModule(name, + var module = ModuleDefinition.CreateModule(name, new ModuleParameters { Kind = ModuleKind.Dll, ReflectionImporterProvider = MMReflectionImporter.ProviderNoDefault @@ -134,20 +154,7 @@ operand is FieldReference fref && new ParameterDefinition("<>_this", ParameterAttributes.None, type.Relink(relinker, clone))); } - foreach (var settingsDumpPath in dumpPaths) - { - var fullPath = Path.GetFullPath(settingsDumpPath); - try - { - Directory.CreateDirectory(fullPath); - using var stream = File.OpenWrite(Path.Combine(fullPath, $"{module.Name}.dll")); - module.Write(stream); - } - catch (Exception e) - { - Logger.Log(Logger.LogChannel.Error, () => $"Failed to dump {md.GetID(simple: true)} to {fullPath}: {e}"); - } - } + return module; } private static string SanitizeTypeName(string typeName) diff --git a/HarmonyTests/Extras/Dump.cs b/HarmonyTests/Extras/Dump.cs new file mode 100644 index 00000000..01b12fee --- /dev/null +++ b/HarmonyTests/Extras/Dump.cs @@ -0,0 +1,36 @@ +using HarmonyLib.Internal.Util; +using HarmonyLibTests; +using MonoMod.Cil; +using MonoMod.Utils; +using NUnit.Framework; +using System; +using System.Linq; + + +namespace HarmonyTests.Extras; + +[TestFixture] +public class Dump : TestLogger +{ + [Test] + public void Test_DumpIlLabel() + { + using var dmd = new DynamicMethodDefinition("DumpIlLabelTestMethod", typeof(void), Type.EmptyTypes); + var il = new ILContext(dmd.Definition); + var cur = new ILCursor(il); + + var label = cur.DefineLabel(); + cur.EmitBr(label); + cur.EmitRet(); + label.Target = cur.Prev; + + var method = cur.Method; + + using var dumpedModule = CecilEmitter.DumpImpl(method); + var dumpedMethod = dumpedModule.Types.SelectMany(i => i.Methods).FirstOrDefault(); + Assert.NotNull(dumpedMethod); + + var instructions = dumpedMethod!.Body.Instructions; + Assert.True(instructions[0].Operand == instructions[1]); + } +}