ILSpy icon indicating copy to clipboard operation
ILSpy copied to clipboard

Unused ref local variable introduced

Open sgkoishi opened this issue 5 years ago • 7 comments

BaseMod.zip BaseMod.BaseDrawing.DrawVectorChain(...)

Result:

if (Jump <= 0f)
{
    Jump = ((float) textures[1].Height - 2f) * scale;
}
float length = 0f;
for (int i = 0; i < chain.Length - 1; i++)
{
    length += Vector2.Distance(chain[i], chain[i + 1]);
}
Vector2 start = chain[0];
Vector2 end = chain[chain.Length - 1];
Vector2 value = end - start;
value.Normalize();
float Way = 0f;
float rotation = BaseUtility.RotationTo(chain[0], chain[1]) - 1.57f;
int num = 0;
int maxTextures = textures.Length - 2;
int num2 = 0;
ref Vector2 reference = ref chain[0];  // <---- Never used
for (; Way < length; Way += Jump)
{

Source:

if (Jump <= 0f)
{ 
    Jump = (textures[1].Height - 2f) * scale; 
}
float length = 0f;	
for (int m = 0; m < chain.Length - 1; m++)
{
	length += Vector2.Distance(chain[m], chain[m + 1]);
}
Vector2 start = chain[0];
Vector2 end = chain[chain.Length - 1];
Vector2 dir = end - start;
dir.Normalize();			
float Way = 0f;
float rotation = BaseUtility.RotationTo(chain[0], chain[1]) - 1.57f;
int texID = 0;
int maxTextures = textures.Length - 2;
int currentChain = 0;
Vector2 lastV = chain[0];
while (Way < length)
{

The variable was poped right after load to evaluation stack.

ldarg.3
ldc.i4.0
ldelema [Microsoft.Xna.Framework]Microsoft.Xna.Framework.Vector2
pop

sgkoishi avatar Aug 05 '19 21:08 sgkoishi

We can't eliminate the array access, because it might throw out-of-bounds exceptions. So our only option to get rid of the ref local would be to introduce an artificial ldobj.

dgrunwald avatar Aug 05 '19 21:08 dgrunwald

We can't eliminate the array access, because it might throw out-of-bounds exceptions. So our only option to get rid of the ref local would be to introduce an artificial ldobj.

The code was compiled to ldelema instead of ldelem. Is it some compiler optimization (does it make any difference)?

sgkoishi avatar Aug 05 '19 21:08 sgkoishi

If the result is not used, there's no difference between ldelema and ldelem. So yes, it looks like a compiler optimization.

It's always unclear whether we should expose optimizations to the user or hide them by de-optimizing the code (see also: #1545). But at least for language versions where the decompiler isn't supposed to use ref locals, we should probably turn the ldelema back into an ldelem to avoid the ref local.

dgrunwald avatar Aug 05 '19 21:08 dgrunwald

I am experiencing this in maybe a different way and I tried to debug this, but without much success. I can't even get the thing to compile with the same IL, but it is a Unity game that I tried to decomp the dll.

This is the IL with the C# commented from avalonia ILSpy:

// ref Vector3 reference4 = ref vectordata[0];
IL_0f6c: ldarg.0
IL_0f6d: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3[] NPCControl::vectordata
IL_0f72: ldc.i4.0
IL_0f73: ldelema [UnityEngine.CoreModule]UnityEngine.Vector3
// Vector3 vector = reference4;
IL_0f78: dup
IL_0f79: ldobj [UnityEngine.CoreModule]UnityEngine.Vector3
// Vector3? startpos3 = entity.startpos;
IL_0f7e: ldarg.0
IL_0f7f: ldfld class EntityControl NPCControl::entity
IL_0f84: ldfld valuetype [mscorlib]System.Nullable`1<valuetype [UnityEngine.CoreModule]UnityEngine.Vector3> EntityControl::startpos
IL_0f89: stloc.s 13
// reference4 = vector + startpos3.Value;
IL_0f8b: ldloca.s 13
IL_0f8d: call instance !0 valuetype [mscorlib]System.Nullable`1<valuetype [UnityEngine.CoreModule]UnityEngine.Vector3>::get_Value()
IL_0f92: call valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 [UnityEngine.CoreModule]UnityEngine.Vector3::op_Addition(valuetype [UnityEngine.CoreModule]UnityEngine.Vector3, valuetype [UnityEngine.CoreModule]UnityEngine.Vector3)
IL_0f97: stobj [UnityEngine.CoreModule]UnityEngine.Vector3

The problem is it doesn't honor the C# version I am sending it which is C# 4 resulting in uncompilable code. I send C# 4 because this game uses a very old Mono runtime that only supported up to this version.

I found that ILSpy does try to inline, but it fails because it thinks the same thing is loaded more than once. Aggressive inlining does not solve this issue. How can one go about addressing this for low C# version? The last comment mentions changing an instruction, but I am not sure I am familiar enough with the codebase to know how and where this could happen.

aldelaro5 avatar Sep 19 '22 23:09 aldelaro5

I am experiencing this in maybe a different way and I tried to debug this, but without much success. I can't even get the thing to compile with the same IL, but it is a Unity game that I tried to decomp the dll.

This is the IL with the C# commented from avalonia ILSpy:

// ref Vector3 reference4 = ref vectordata[0];
IL_0f6c: ldarg.0
IL_0f6d: ldfld valuetype [UnityEngine.CoreModule]UnityEngine.Vector3[] NPCControl::vectordata
IL_0f72: ldc.i4.0
IL_0f73: ldelema [UnityEngine.CoreModule]UnityEngine.Vector3
// Vector3 vector = reference4;
IL_0f78: dup
IL_0f79: ldobj [UnityEngine.CoreModule]UnityEngine.Vector3
// Vector3? startpos3 = entity.startpos;
IL_0f7e: ldarg.0
IL_0f7f: ldfld class EntityControl NPCControl::entity
IL_0f84: ldfld valuetype [mscorlib]System.Nullable`1<valuetype [UnityEngine.CoreModule]UnityEngine.Vector3> EntityControl::startpos
IL_0f89: stloc.s 13
// reference4 = vector + startpos3.Value;
IL_0f8b: ldloca.s 13
IL_0f8d: call instance !0 valuetype [mscorlib]System.Nullable`1<valuetype [UnityEngine.CoreModule]UnityEngine.Vector3>::get_Value()
IL_0f92: call valuetype [UnityEngine.CoreModule]UnityEngine.Vector3 [UnityEngine.CoreModule]UnityEngine.Vector3::op_Addition(valuetype [UnityEngine.CoreModule]UnityEngine.Vector3, valuetype [UnityEngine.CoreModule]UnityEngine.Vector3)
IL_0f97: stobj [UnityEngine.CoreModule]UnityEngine.Vector3

The problem is it doesn't honor the C# version I am sending it which is C# 4 resulting in uncompilable code. I send C# 4 because this game uses a very old Mono runtime that only supported up to this version.

I found that ILSpy does try to inline, but it fails because it thinks the same thing is loaded more than once. Aggressive inlining does not solve this issue. How can one go about addressing this for low C# version? The last comment mentions changing an instruction, but I am not sure I am familiar enough with the codebase to know how and where this could happen.

If all you want is C# code, this snippet is basically equivalent to this.vectordata[0] += this.entity.startpos.Value. The compiler did some magic and compile it to some "not so straight forward" IL, and it is not quite easy to analyze.

sgkoishi avatar Sep 19 '22 23:09 sgkoishi

oh ik what it should look like, but this error is actually the only bug ILSpy has on this entire binary and it appears twice so I wanted to see if I could help fix it. Doesn't seem like it, but I was just putting it there in case.

aldelaro5 avatar Sep 20 '22 00:09 aldelaro5

oh ik what it should look like, but this error is actually the only bug ILSpy has on this entire binary and it appears twice so I wanted to see if I could help fix it. Doesn't seem like it, but I was just putting it there in case.

This is actually more related to #1610 and we actually upgrade the C# version to let it compiles :joy:

sgkoishi avatar Sep 22 '22 16:09 sgkoishi