Skip to content

Commit beb29dc

Browse files
CedaryCatCedaryCat
committed
fix(analysis): detect atomic ref mutations and value-type address flows
- Treat Volatile.Write and Interlocked ref operations as mutations - Track value-type address loads (ldarga/ldloca/ldflda/ldsflda) in flow/reference analysis
1 parent 1096483 commit beb29dc

6 files changed

Lines changed: 133 additions & 19 deletions

File tree

‎src/OTAPI.UnifiedServerProcess/Core/Analysis/ParamModificationAnalysis/ParamModificationAnalyzer.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,28 @@ static bool TryAddModifications(Dictionary<string, Dictionary<int, ParameterMuta
227227
return result;
228228
}
229229

230+
static bool IsAtomicModificationMethod(MethodReference callee) {
231+
if (callee.Parameters.Count == 0 || callee.Parameters[0].ParameterType is not ByReferenceType) {
232+
return false;
233+
}
234+
235+
if (callee.DeclaringType.FullName == typeof(System.Threading.Volatile).FullName) {
236+
return callee.Name is "Write";
237+
}
238+
239+
if (callee.DeclaringType.FullName == typeof(System.Threading.Interlocked).FullName) {
240+
return callee.Name is "Exchange"
241+
or "CompareExchange"
242+
or "Increment"
243+
or "Decrement"
244+
or "Add"
245+
or "And"
246+
or "Or";
247+
}
248+
249+
return false;
250+
}
251+
230252
void HandleModifyField(Instruction instruction) {
231253
FieldReference field = (FieldReference)instruction.Operand;
232254
foreach (var path in MonoModCommon.Stack.AnalyzeInstructionArgsSources(method, instruction, jumpSites)) {
@@ -325,6 +347,32 @@ void HandleModifyCollectionElement(ParameterUsageTrack tracedMethodData, MonoMod
325347
}
326348
}
327349
}
350+
void HandleModifyAtomicOperation(ParameterUsageTrack tracedMethodData, MonoModCommon.Stack.StackTopTypePath[][] loadParamsInEveryPaths) {
351+
const int atomicRefParameterIndex = 0;
352+
353+
foreach (var paramGroup in loadParamsInEveryPaths) {
354+
if (paramGroup.Length <= atomicRefParameterIndex) {
355+
continue;
356+
}
357+
358+
var loadModifyingInstance = paramGroup[atomicRefParameterIndex];
359+
360+
// If loadModifyingInstance.RealPushValueInstruction is not coming from the Mutations of caller method, skip
361+
if (!tracedMethodData.StackValueTraces.TryGetTrace(ParameterUsageTrack.GenerateStackKey(method, loadModifyingInstance.RealPushValueInstruction), out var tracedStackData)) {
362+
continue;
363+
}
364+
365+
foreach (var modifiedParameter in tracedStackData.ReferencedParameters.Values) {
366+
// Ignore the ModificationAccessPath of "this" in constructors, we only care about input parameters.
367+
if (modifiedParameter.TracedParameter.IsParameterThis(method) && method.IsConstructor) {
368+
continue;
369+
}
370+
if (CheckAndAddModifications(modifiedParametersAllMethods, processingMethodId, method, modifiedParameter)) {
371+
hasExternalChange = true;
372+
}
373+
}
374+
}
375+
}
328376
void HandleMethodCall(Instruction instruction) {
329377

330378
MethodReference callee = (MethodReference)instruction.Operand;
@@ -361,6 +409,10 @@ void HandleMethodCall(Instruction instruction) {
361409
HandleModifyCollectionElement(tracedMethodData, loadParamsInEveryPaths, instruction);
362410
return;
363411
}
412+
if (IsAtomicModificationMethod(callee)) {
413+
HandleModifyAtomicOperation(tracedMethodData, loadParamsInEveryPaths);
414+
return;
415+
}
364416

365417
//// We assume that if "this" is modified in invocation,
366418
//// it's equivalent to Mutations "this" has modified when the delegate is created

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,11 @@ private void ProcessMethod(
179179
case Code.Ldarg_3:
180180
case Code.Ldarg_S:
181181
case Code.Ldarg:
182+
HandleLoadArgument(instruction, false);
183+
break;
182184
case Code.Ldarga_S:
183185
case Code.Ldarga:
184-
HandleLoadArgument(instruction);
186+
HandleLoadArgument(instruction, true);
185187
break;
186188

187189
case Code.Stloc_0:
@@ -199,14 +201,18 @@ private void ProcessMethod(
199201
case Code.Ldloc_3:
200202
case Code.Ldloc_S:
201203
case Code.Ldloc:
204+
HandleLoadLocal(instruction, false);
205+
break;
202206
case Code.Ldloca_S:
203207
case Code.Ldloca:
204-
HandleLoadLocal(instruction);
208+
HandleLoadLocal(instruction, true);
205209
break;
206210

207211
case Code.Ldfld:
212+
HandleLoadField(instruction, false);
213+
break;
208214
case Code.Ldflda:
209-
HandleLoadField(instruction);
215+
HandleLoadField(instruction, true);
210216
break;
211217

212218
case Code.Stfld:
@@ -255,9 +261,9 @@ void HandleReturnInstruction(Instruction instruction) {
255261
}
256262
}
257263

258-
void HandleLoadArgument(Instruction instruction) {
264+
void HandleLoadArgument(Instruction instruction, bool isAddress) {
259265
var parameter = MonoModCommon.IL.GetReferencedParameter(method, instruction);
260-
if (parameter.ParameterType.IsTruelyValueType()) return;
266+
if (parameter.ParameterType.IsTruelyValueType() && !isAddress) return;
261267

262268
ParameterTracingChain chain = new ParameterTracingChain(parameter, [], []);
263269
var stackKey = ParameterUsageTrack.GenerateStackKey(method, instruction);
@@ -285,9 +291,9 @@ void HandleStoreLocal(Instruction instruction) {
285291
}
286292
}
287293

288-
void HandleLoadLocal(Instruction instruction) {
294+
void HandleLoadLocal(Instruction instruction, bool isAddress) {
289295
var variable = MonoModCommon.IL.GetReferencedVariable(method, instruction);
290-
if (variable.VariableType.IsTruelyValueType()) return;
296+
if (variable.VariableType.IsTruelyValueType() && !isAddress) return;
291297

292298
if (localTrace.TryGetTrace(variable, out var trace)) {
293299
var stackKey = ParameterUsageTrack.GenerateStackKey(method, instruction);
@@ -297,9 +303,9 @@ void HandleLoadLocal(Instruction instruction) {
297303
}
298304
}
299305

300-
void HandleLoadField(Instruction instruction) {
306+
void HandleLoadField(Instruction instruction, bool isAddress) {
301307
FieldReference fieldRef = (FieldReference)instruction.Operand;
302-
if (fieldRef.FieldType.IsTruelyValueType()) return;
308+
if (fieldRef.FieldType.IsTruelyValueType() && !isAddress) return;
303309

304310
foreach (var path in MonoModCommon.Stack.AnalyzeInstructionArgsSources(method, instruction, jumpSites)) {
305311
foreach (var instanceLoad in MonoModCommon.Stack.AnalyzeStackTopTypeAllPaths(method, path.ParametersSources[0].Instructions.Last(), jumpSites)) {

‎src/OTAPI.UnifiedServerProcess/Core/Analysis/StaticFieldModificationAnalysis/StaticFieldModificationAnalyzer.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,28 @@ static void AddField(Dictionary<string, FieldDefinition> dict, FieldDefinition f
123123
dict.TryAdd(field.GetIdentifier(), field);
124124
}
125125

126+
static bool IsAtomicModificationMethod(MethodReference callee) {
127+
if (callee.Parameters.Count == 0 || callee.Parameters[0].ParameterType is not ByReferenceType) {
128+
return false;
129+
}
130+
131+
if (callee.DeclaringType.FullName == typeof(System.Threading.Volatile).FullName) {
132+
return callee.Name is "Write";
133+
}
134+
135+
if (callee.DeclaringType.FullName == typeof(System.Threading.Interlocked).FullName) {
136+
return callee.Name is "Exchange"
137+
or "CompareExchange"
138+
or "Increment"
139+
or "Decrement"
140+
or "Add"
141+
or "And"
142+
or "Or";
143+
}
144+
145+
return false;
146+
}
147+
126148
staticFieldReferenceAnalyzer.AnalyzedMethods.TryGetValue(caller.GetIdentifier(), out StaticFieldUsageTrack? staticFieldReferenceData);
127149

128150
Dictionary<Instruction, List<Instruction>> jumpSites = this.GetMethodJumpSites(caller);
@@ -200,6 +222,9 @@ static void AddField(Dictionary<string, FieldDefinition> dict, FieldDefinition f
200222
// The called method is a collection method
201223
|| (resolvedCallee is not null && CollectionElementLayer.IsModificationMethod(typeInheritanceGraph, caller, instruction))
202224

225+
// The called method performs atomic modification to the ref target
226+
|| IsAtomicModificationMethod(methodRef)
227+
203228
// The called method return a reference
204229
|| (methodRef.ReturnType is ByReferenceType && methodRef.Name is "get_Item")) {
205230

‎src/OTAPI.UnifiedServerProcess/Core/Analysis/StaticFieldReferenceAnalysis/StaticFieldReferenceAnalyzer.cs‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -186,19 +186,25 @@ private void ProcessMethod(
186186
case Code.Ldloc_3:
187187
case Code.Ldloc_S:
188188
case Code.Ldloc:
189+
HandleLoadLocal(instruction, false);
190+
break;
189191
case Code.Ldloca_S:
190192
case Code.Ldloca:
191-
HandleLoadLocal(instruction);
193+
HandleLoadLocal(instruction, true);
192194
break;
193195

194196
case Code.Ldsfld:
197+
HandleLoadStaticField(instruction, false);
198+
break;
195199
case Code.Ldsflda:
196-
HandleLoadStaticField(instruction);
200+
HandleLoadStaticField(instruction, true);
197201
break;
198202

199203
case Code.Ldfld:
204+
HandleLoadField(instruction, false);
205+
break;
200206
case Code.Ldflda:
201-
HandleLoadField(instruction);
207+
HandleLoadField(instruction, true);
202208
break;
203209

204210
case Code.Stfld:
@@ -247,10 +253,10 @@ void HandleReturnInstruction(Instruction instruction) {
247253
}
248254
}
249255

250-
void HandleLoadStaticField(Instruction instruction) {
256+
void HandleLoadStaticField(Instruction instruction, bool isAddress) {
251257
var fieldDef = ((FieldReference)instruction.Operand).TryResolve();
252258
if (fieldDef is null) return;
253-
if (fieldDef.FieldType.IsTruelyValueType()) return;
259+
if (fieldDef.FieldType.IsTruelyValueType() && !isAddress) return;
254260

255261
var stackKey = StaticFieldUsageTrack.GenerateStackKey(method, instruction);
256262

@@ -277,9 +283,9 @@ void HandleStoreLocal(Instruction instruction) {
277283
}
278284
}
279285

280-
void HandleLoadLocal(Instruction instruction) {
286+
void HandleLoadLocal(Instruction instruction, bool isAddress) {
281287
var variable = MonoModCommon.IL.GetReferencedVariable(method, instruction);
282-
if (variable.VariableType.IsTruelyValueType()) return;
288+
if (variable.VariableType.IsTruelyValueType() && !isAddress) return;
283289

284290
// If loading a traced variable to the stack, then we need to update the stack trace
285291
if (localTrace.TryGetTrace(variable, out var trace)) {
@@ -290,9 +296,9 @@ void HandleLoadLocal(Instruction instruction) {
290296
}
291297
}
292298

293-
void HandleLoadField(Instruction instruction) {
299+
void HandleLoadField(Instruction instruction, bool isAddress) {
294300
FieldReference fieldRef = (FieldReference)instruction.Operand;
295-
if (fieldRef.FieldType.IsTruelyValueType()) return;
301+
if (fieldRef.FieldType.IsTruelyValueType() && !isAddress) return;
296302

297303
foreach (var path in MonoModCommon.Stack.AnalyzeInstructionArgsSources(method, instruction, jumpSites)) {
298304
foreach (var instanceLoad in MonoModCommon.Stack.AnalyzeStackTopTypeAllPaths(method, path.ParametersSources[0].Instructions.Last(), jumpSites)) {

‎src/OTAPI.UnifiedServerProcess/Core/Patching/FieldFilterPatching/InitialFieldModificationProcessor.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,28 @@ void AddFieldModification(MethodDefinition method, Dictionary<string, HashSet<In
150150
}
151151
}
152152

153+
static bool IsAtomicModificationMethod(MethodReference callee) {
154+
if (callee.Parameters.Count == 0 || callee.Parameters[0].ParameterType is not ByReferenceType) {
155+
return false;
156+
}
157+
158+
if (callee.DeclaringType.FullName == typeof(System.Threading.Volatile).FullName) {
159+
return callee.Name is "Write";
160+
}
161+
162+
if (callee.DeclaringType.FullName == typeof(System.Threading.Interlocked).FullName) {
163+
return callee.Name is "Exchange"
164+
or "CompareExchange"
165+
or "Increment"
166+
or "Decrement"
167+
or "Add"
168+
or "And"
169+
or "Or";
170+
}
171+
172+
return false;
173+
}
174+
153175
foreach (Instruction? instruction in method.Body.Instructions) {
154176

155177
switch (instruction.OpCode.Code) {
@@ -226,6 +248,9 @@ void AddFieldModification(MethodDefinition method, Dictionary<string, HashSet<In
226248
// The called method is a collection method
227249
|| (resolvedCallee is not null && CollectionElementLayer.IsModificationMethod(typeInheritanceGraph, method, instruction))
228250

251+
// The called method performs atomic modification to the ref target
252+
|| IsAtomicModificationMethod(calleeRef)
253+
229254
// The called method return a reference
230255
|| (calleeRef.ReturnType is ByReferenceType && calleeRef.Name is "get_Item")) {
231256

‎src/OTAPI.UnifiedServerProcess/OTAPI.UnifiedServerProcess.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
<ItemGroup>
2222
<PackageReference Include="OTAPI.Upcoming" Version="3.3.7-mf-a807e26.13" />
2323
<PackageReference Include="MonoMod.RuntimeDetour.HookGen" Version="22.7.31.1" />
24-
<PackageReference Include="ModFramework.Modules.CSharp" Version="1.1.15-a807e26.13" />
24+
<PackageReference Include="ModFramework.Modules.CSharp" Version="1.1.15-mf-a807e26.13" />
2525
</ItemGroup>
2626

2727
<ItemGroup>

0 commit comments

Comments
 (0)