Hi
Thanks for your nice sample. I'm completely agree with you. I've
checked the sample with two versions of Mono.Cecil.
For the first test I took original Mono.Cecil without a patch. Here
are the funny results:
Before L4 : 2
==> ==> Before L6 = 0
But we can't assume anything here about After L4
For the second test I applied the patch. Here are results:
Before L4 : 2
==> ==> Before L6 = 2
But we can't assume anything here about After L4
As you can see, the original Mono.Cecil algorithm is broken. The only
reason why it is not so critical in the most of applications is that
because this bug is almost always masked by neighbor code flow branch.
In your sample neighbor branch is L2, L5, L6 and it gives stack size
of 2 for L6. But when such neighbor branch is absent then Mono.Cecil
algorithm is going insane because it assumes stack size 0 before L6
and after L4 jump.
Your sample gave me a good understanding that there can be a lot of
neighbor branches and I've found another weak place in the existing
algorithm. It does not respect the possibility of existence of several
parallel neighbor flow branches that can have different stack depths.
It is very uncommon case which is never produced by known .NET
compilers. However such a case is possible in raw CIL and maybe some
future compilers. So I improved algorithm in this area. The
corresponding patch is below.
Index: CodeWriter.cs
===================================================================
--- CodeWriter.cs (revision 130916)
+++ CodeWriter.cs (working copy)
@@ -469,16 +469,15 @@
switch (instr.OpCode.OperandType) {
case OperandType.InlineBrTarget:
case OperandType.ShortInlineBrTarget:
- m_stackSizes [instr.Operand] =
current;
- break;
+
UpdateForwardStackSize(instr.Operand, current);
+ break;
case OperandType.InlineSwitch:
foreach (Instruction target in
(Instruction []) instr.Operand)
- m_stackSizes [target] =
current;
+
UpdateForwardStackSize(target, current);
break;
}
switch (instr.OpCode.FlowControl) {
- case FlowControl.Branch:
case FlowControl.Throw:
case FlowControl.Return:
// next statement is not reachable from
this statement, so reset
the stack depth to 0
@@ -490,6 +489,14 @@
instructions.Container.MaxStack = max + 1; // you never
know
}
+ void UpdateForwardStackSize(object instr, int newStackSize)
+ {
+ object existingStackSize = m_stackSizes[instr];
+ if (existingStackSize != null)
+ newStackSize = Math.Max((int)existingStackSize,
newStackSize);
+ m_stackSizes[instr] = newStackSize;
+ }
+
static int GetPushDelta (Instruction instruction)
{
OpCode code = instruction.OpCode;
On 3 Кві, 12:37, Lotfi Gheribi <[email protected]> wrote:
> Have a look at this C# code:
>
> bool var0;
> int var1 = 3 + (var0?0:1);
>
> This would be translated to :
>
> L0: ldc.i4.3
> L1: ldloc.0
> L2: brfalse.s L5
> L3: ldc.i4.0
> L4: br.s L6
> L5: ldc.i4.1
> L6 : add
> L7: stloc.1
> L8 :
>
> According to your correction, I assume that the stack level at the
> beginning of L6 should be 0, because it's just after a br.s ! But in
> fact it's 1 ! (The code provided above is 100% valid, and does work on
> mono of course)
>
> I believe the right algorithm should process the code as follows :
>
> Before L0 : 0
> ==> After L0 : 1
>
> Before L1 : 1
> ==> After L1 : 2
>
> Before L2 : 2
> ==> After L2 : 1 ==> Before L5 = 1
>
> Before L3 : 1
> ==> After L3 : 2
>
> Before L4 : 2
> ==> ==> Before L6 = 2
> But we can't assume anything here about After L4
>
> Before L5 = 1
> ==> After L5 = 2
>
> Before L6 = 2
> ==> After L6 = 1
>
> Before L7 = 1
> ==> After L7 = 0
>
> Correct me if I'm wrong
--~--~---------~--~----~------------~-------~--~----~
--
mono-cecil
-~----------~----~----~----~------~----~------~--~---
