Skip to content

Commit 458ff64

Browse files
committed
fix(patching): harden IL merge and delegate retargeting
- retarget direct delegate invoke calls to generated placeholder delegates - ignore synthetic constructor patches when target types have no default ctor - replace LINQ/lambda-based IL scans with explicit instruction walks to prevent compiler-generated helper types and methods without MonoModIgnore from being merged into the output assembly - add a DEBUG metadata importer hook to inspect unexpected assembly references introduced during IL merge - remove generated Program before writing the merged module - bump TrProtocol submodule
1 parent 3802033 commit 458ff64

File tree

10 files changed

+106
-42
lines changed

10 files changed

+106
-42
lines changed

src/OTAPI.UnifiedServerProcess/Core/Analysis/ParameterFlowAnalysis/ParameterFlowAnalyzer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ void HandleLoadCollectionElement(Instruction instruction) {
547547

548548
void HandleStoreCollectionElement(Instruction instruction, int indexOfValueInArguments) {
549549
if (instruction.Offset is 155 && method.GetIdentifier() == "Terraria.GameContent.Generation.Dungeon.LayoutProviders.DualDungeonLayoutProvider/HallwayCalculator.MakeHall(Terraria.GameContent.Generation.Dungeon.LayoutProviders.DualDungeonLayoutProvider/HallwayCalculator/HallLine,Terraria.GameContent.Generation.Dungeon.Halls.DungeonHallType)") {
550-
// Debug
550+
// DEBUG
551551
}
552552
foreach (var path in MonoModCommon.Stack.AnalyzeParametersSources(method, instruction, jumpSites)) {
553553
var instancePath = MonoModCommon.Stack.AnalyzeStackTopTypeAllPaths(method, path.ParametersSources[0].Instructions.Last(), jumpSites)

src/OTAPI.UnifiedServerProcess/Core/Patching/GeneralPatching/Arguments/DelegatePlaceholderProcessor.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,14 @@ private void PropagateFromThisInfected(
13671367
Stack<ValueSeed> valueWork,
13681368
Stack<MethodSeedWorkItem> globalWork) {
13691369

1370-
if (calleeDef is null || calleeDef.Module != module) {
1370+
// Direct delegate calls such as `oldDelegate.Invoke(...)` need the callee itself
1371+
// retargeted to the generated placeholder delegate, even when the old delegate
1372+
// type lives in this module.
1373+
bool shouldRetargetDirectDelegateCall =
1374+
calleeRef.DeclaringType is not null
1375+
&& infectedThis.Transform.OldDelegateFullNames.Contains(calleeRef.DeclaringType.FullName);
1376+
1377+
if (shouldRetargetDirectDelegateCall || calleeDef is null || calleeDef.Module != module) {
13711378
if (TryRetargetCallToTransformedDeclaringType(callInst, calleeRef, infectedThis.Transform)) {
13721379
calleeRef = (MethodReference)callInst.Operand;
13731380
calleeDef = calleeRef.TryResolve();
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+

2+
using Mono.Cecil;
3+
using System.Runtime.CompilerServices;
4+
5+
namespace OTAPI.UnifiedServerProcess
6+
{
7+
public static class DEBUG
8+
{
9+
public static void Load(ModuleDefinition module) {
10+
metadata_importer(module) = new DebugMetadataImporter(module);
11+
}
12+
13+
[UnsafeAccessor(UnsafeAccessorKind.Field, Name = "metadata_importer")]
14+
static extern ref IMetadataImporter metadata_importer(ModuleDefinition module);
15+
16+
class DebugMetadataImporter(ModuleDefinition module) : DefaultMetadataImporter(module)
17+
{
18+
public override AssemblyNameReference ImportReference(AssemblyNameReference name) {
19+
return base.ImportReference(name);
20+
}
21+
}
22+
}
23+
}

src/OTAPI.UnifiedServerProcess/ModAssemblyMerger.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Mono.Cecil.Rocks;
55
using Mono.Collections.Generic;
66
using MonoMod.Utils;
7+
using Newtonsoft.Json.Linq;
78
using OTAPI.UnifiedServerProcess.Extensions;
89
using System;
910
using System.Collections.Generic;
@@ -330,22 +331,30 @@ void PrepareMethod(TypeDefinition targetType, MethodDefinition modMethod, Method
330331
instCount += 1;
331332
}
332333
}
333-
if (instCount <= 3 && originalMethod is not null) {
334+
335+
if (instCount <= 3 && (originalMethod is not null || shouldBeIgnore(targetType, modMethod))) {
334336
if (!ignored) {
335337
ignored = true;
336-
TypeReference attType_ctor = modMethod.Module.ImportReference(typeof(MonoMod.MonoModIgnore));
338+
TypeReference attType_ctor = targetType.Module.ImportReference(typeof(MonoMod.MonoModIgnore));
337339
modMethod.CustomAttributes.Add(new CustomAttribute(new MethodReference(".ctor", modMethod.Module.TypeSystem.Void, attType_ctor) { HasThis = true }));
338340
}
339341
}
340342
else {
341-
TypeReference attType_ctor = modMethod.Module.ImportReference(typeof(MonoMod.MonoModConstructor));
343+
TypeReference attType_ctor = targetType.Module.ImportReference(typeof(MonoMod.MonoModConstructor));
342344
modMethod.CustomAttributes.Add(new CustomAttribute(new MethodReference(".ctor", modMethod.Module.TypeSystem.Void, attType_ctor) { HasThis = true }));
343345
}
344346
}
345347
if (!ignored && originalMethod is not null && IgnoreExistingMethods.Contains(modMethod.Name)) {
346-
TypeReference attType_ctor = modMethod.Module.ImportReference(typeof(MonoMod.MonoModIgnore));
348+
TypeReference attType_ctor = targetType.Module.ImportReference(typeof(MonoMod.MonoModIgnore));
347349
modMethod.CustomAttributes.Add(new CustomAttribute(new MethodReference(".ctor", modMethod.Module.TypeSystem.Void, attType_ctor) { HasThis = true }));
348350
}
351+
352+
static bool shouldBeIgnore(TypeDefinition targetType, MethodDefinition modMethod) {
353+
return
354+
modMethod.Parameters.Count is 0 &&
355+
!targetType.Methods.Any(m => m is { IsConstructor: true, IsStatic: false, Parameters: [] }) &&
356+
targetType.Methods.Any(m => m.IsConstructor && !m.IsStatic);
357+
}
349358
}
350359
static void SetMemberReplace(ModuleDefinition module, Collection<CustomAttribute> attributes, bool isEnum) {
351360
Type type = isEnum ? typeof(MonoMod.MonoModEnumReplace) : typeof(MonoMod.MonoModReplace);

src/OTAPI.UnifiedServerProcess/Mods/MessageBufferMod.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using OTAPI.UnifiedServerProcess.Extensions;
99
using System;
1010
using System.IO;
11-
using System.Linq;
1211

1312
[Modification(ModType.PostMerge, "Add overload of GetData", ModPriority.Early)]
1413
[MonoMod.MonoModIgnore]
@@ -85,7 +84,11 @@ void PatchMessageBuffer(ModFwModder modder) {
8584
body.Add(Instruction.Create(OpCodes.Ldarg_0));
8685
body.Add(Instruction.Create(OpCodes.Ldfld, new FieldReference("reader", param_reader.ParameterType, messageBufferTypeDef)));
8786
var overloadRef = new MethodReference(overload.Name, overload.ReturnType, overload.DeclaringType) { HasThis = overload.HasThis };
88-
overloadRef.Parameters.AddRange(overload.Parameters.Select(p => new ParameterDefinition(p.ParameterType)));
87+
88+
foreach (var p in overload.Parameters) {
89+
overloadRef.Parameters.Add(new ParameterDefinition(p.ParameterType));
90+
}
91+
8992
body.Add(Instruction.Create(OpCodes.Call, overloadRef));
9093
body.Add(Instruction.Create(OpCodes.Ret));
9194
}

src/OTAPI.UnifiedServerProcess/Mods/NetplayMod.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Mono.Cecil.Rocks;
77
using MonoMod.Cil;
88
using System.Collections.Generic;
9-
using System.Linq;
109
using System.Runtime.InteropServices;
1110
using Terraria;
1211

@@ -20,9 +19,12 @@ void NetplayConnectionCheck(ModFwModder modder) {
2019
if (!method.HasBody || method.Name == nameof(NetMessage.CheckCanSend)) {
2120
continue;
2221
}
23-
if (method.Body.Instructions.Any(inst => inst is { Operand: MemberReference { Name: "IsConnected", DeclaringType.Name: "RemoteClient" } })) {
24-
methods.Add(method);
25-
continue;
22+
23+
foreach (var inst in method.Body.Instructions) {
24+
if (inst is { Operand: MemberReference { Name: "IsConnected", DeclaringType.Name: "RemoteClient" } }) {
25+
methods.Add(method);
26+
break;
27+
}
2628
}
2729
}
2830
}

src/OTAPI.UnifiedServerProcess/Mods/RemoteClientMod.cs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
using OTAPI.UnifiedServerProcess.Commons;
88
using OTAPI.UnifiedServerProcess.Extensions;
99
using System;
10-
using System.Linq;
10+
using System.Collections.Generic;
1111
using Terraria;
1212

1313
[Modification(ModType.PostMerge, "Rewrite RemoteClient ResetSections", ModPriority.Early)]
@@ -17,26 +17,29 @@ void NetplayConnectionCheck(ModFwModder modder) {
1717

1818
TypeDefinition remoteClientDef = modder.Module.GetType("Terraria.RemoteClient");
1919
MethodDefinition mfwh_orig_ResetMDef = remoteClientDef.GetMethod("mfwh_orig_Reset");
20-
System.Collections.Generic.Dictionary<Instruction, System.Collections.Generic.List<Instruction>> jumpSites = MonoModCommon.Stack.BuildJumpSitesMap(mfwh_orig_ResetMDef);
20+
var jumpSites = MonoModCommon.Stack.BuildJumpSitesMap(mfwh_orig_ResetMDef);
2121

22-
Instruction[] clearArrayInst = mfwh_orig_ResetMDef.Body.Instructions.Select(inst => {
22+
List<Instruction> clearArrayInst = [];
23+
foreach (var inst in mfwh_orig_ResetMDef.Body.Instructions) {
2324
if (inst is not Instruction { OpCode.Code: Code.Call, Operand: MethodReference { DeclaringType.FullName: "System.Array", Name: "Clear" } }) {
24-
return null;
25+
continue;
2526
}
2627
MonoModCommon.Stack.FlowPath<MonoModCommon.Stack.ParameterSource>[] path = MonoModCommon.Stack.AnalyzeParametersSources(mfwh_orig_ResetMDef, inst, jumpSites);
2728
if (path.Length != 1) {
28-
return null;
29+
continue;
2930
}
30-
if (path[0].ParametersSources[0].Instructions.Last() is not Instruction {
31+
if (path[0].ParametersSources[0].Instructions[^1] is not Instruction {
3132
OpCode.Code: Code.Ldfld,
3233
Operand: FieldReference { Name: nameof(RemoteClient.TileSections) or nameof(RemoteClient.TileSectionsCheckTime) }
3334
}) {
34-
return null;
35+
continue;
3536
}
3637

37-
return path[0].ParametersSources.SelectMany(x => x.Instructions).Append(inst);
38-
39-
}).SelectMany(x => x ?? []).ToArray();
38+
foreach (var s in path[0].ParametersSources) {
39+
clearArrayInst.AddRange(s.Instructions);
40+
}
41+
clearArrayInst.Add(inst);
42+
}
4043

4144
foreach (Instruction? inst in clearArrayInst) {
4245
mfwh_orig_ResetMDef.Body.Instructions.Remove(inst);

src/OTAPI.UnifiedServerProcess/Mods/SectionsFieldMod.cs

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,45 @@
1111
[Modification(ModType.PostMerge, "Reset Sections aware array when reset world size", ModPriority.Early)]
1212
[MonoMod.MonoModIgnore]
1313
void PatchSectionsAwareFields(ModFwModder modder) {
14+
new SectionMod(modder).Run();
15+
}
1416

15-
ModuleDefinition module = modder.Module;
16-
17-
MethodDefinition resetAndResize = module
18-
.GetType("UnifiedServerProcess.SectionsHelper")
19-
.GetMethod("ResetAndResize");
2017

21-
MethodDefinition setWorldSize = module
22-
.GetType("Terraria.WorldGen")
23-
.GetMethod("mfwh_setWorldSize");
18+
[MonoMod.MonoModIgnore]
19+
class SectionMod
20+
{
21+
readonly ModFwModder modder;
22+
readonly MethodDefinition resetAndResize;
23+
readonly MethodDefinition setWorldSize;
24+
readonly Instruction ret;
25+
public SectionMod(ModFwModder modder) {
26+
this.modder = modder;
27+
var module = modder.Module;
28+
resetAndResize = module
29+
.GetType("UnifiedServerProcess.SectionsHelper")
30+
.GetMethod("ResetAndResize");
31+
setWorldSize = module
32+
.GetType("Terraria.WorldGen")
33+
.GetMethod("mfwh_setWorldSize");
34+
ret = setWorldSize.Body.Instructions.Single(i => i.OpCode.Code is Code.Ret);
35+
}
2436

25-
Instruction ret = setWorldSize.Body.Instructions.Single(i => i.OpCode.Code is Code.Ret);
37+
public void Run() {
2638

27-
MethodDefinition activeSections_Reset = module
28-
.GetType("Terraria.DataStructures.ActiveSections")
29-
.GetMethod("Reset");
30-
Process(activeSections_Reset, "LastActiveTime");
31-
setWorldSize.Body.GetILProcessor()
32-
.InsertBefore(ret, Instruction.Create(OpCodes.Call, MonoModCommon.Structure.CreateMethodReference(activeSections_Reset, activeSections_Reset)));
39+
ModuleDefinition module = modder.Module;
3340

34-
MethodDefinition leashedEntityClear = module
35-
.GetType("Terraria.GameContent.LeashedEntity")
36-
.GetMethod("Clear");
37-
Process(leashedEntityClear, "BySection");
41+
MethodDefinition activeSections_Reset = module
42+
.GetType("Terraria.DataStructures.ActiveSections")
43+
.GetMethod("Reset");
44+
Process(activeSections_Reset, "LastActiveTime");
45+
setWorldSize.Body.GetILProcessor()
46+
.InsertBefore(ret, Instruction.Create(OpCodes.Call, MonoModCommon.Structure.CreateMethodReference(activeSections_Reset, activeSections_Reset)));
3847

48+
MethodDefinition leashedEntityClear = module
49+
.GetType("Terraria.GameContent.LeashedEntity")
50+
.GetMethod("Clear");
51+
Process(leashedEntityClear, "BySection");
52+
}
3953

4054
void Process(MethodDefinition method, string fieldName) {
4155
foreach (Instruction? inst in method.Body.Instructions) {

src/OTAPI.UnifiedServerProcess/PatchExecutor.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public bool Patch(DirectoryInfo outputDir) {
117117
modder.AddTask<CoreLibRelinker>();
118118
}
119119
else if (modType == ModType.Read) {
120+
// DEBUG.Load(modder.Module);
120121
}
121122
else if (modType == ModType.PreWrite) {
122123
PatchingLogic.Patch(logger, modder.Module);
@@ -142,6 +143,8 @@ public bool Patch(DirectoryInfo outputDir) {
142143
mm.MapDependencies();
143144
mm.AutoPatch();
144145

146+
mm.Module.Types.Remove(mm.Module.Types.Single(t => t.FullName is "Program"));
147+
145148
Console.WriteLine($"[OTAPI-ProC] Writing: {status}, Path={new Uri(Environment.CurrentDirectory).MakeRelativeUri(new(mm.OutputPath))}");
146149

147150
mm.Write();

0 commit comments

Comments
 (0)