mparlak / Flee

Fast Lightweight Expression Evaluator
607 stars 119 forks source link

System.NotSupportedException: 'Illegal one-byte branch at position: 233. Requested branch was: 204.' #86

Open suriyadi15 opened 3 years ago

suriyadi15 commented 3 years ago

I use .NET Core 2.0 and this is the code:

class Program
{
    static void Main(string[] args)
    {
        ExpressionContext context = new ExpressionContext();

        context.Imports.AddType(typeof(AggregationOperation));

        //this not work
        string processedFormula = "IF(2.1<>2.1,IF(2.1>2.1,2.1,IF(AND(2.1>2.1,2.1<=2.1),2.1,IF(AND(2.1>2.1,2.1<=2.1),2.1,2.1))),IF(2.1>2.1,2.1,IF(AND(2.1>2.1,2.1<=2.1),2.1,IF(AND(2.1>2.1,2.1<=2.1),2.1,2.1))))";

        //this work
        //string processedFormula = "IF(2<>2,IF(2>2,2,IF(AND(2>2,2<=2),2,IF(AND(2>2,2<=2),2,2))),IF(2>2,2,IF(AND(2>2,2<=2),2,IF(AND(2>2,2<=2),2,2))))";

        processedFormula = Regex.Replace(processedFormula, @"and\(", "andF(", RegexOptions.IgnoreCase);

        var e = context.CompileDynamic(processedFormula);
        object result = e.Evaluate();

        System.Console.ReadKey();
    }
}

public static class AggregationOperation
{
    public static bool andF(params bool[] conditions)
    {
        bool result = true;

        foreach (var item in conditions)
        {
            result = result && item;
        }

        return result;
    }
}

The exception throw

System.NotSupportedException
  HResult=0x80131515
  Message=Illegal one-byte branch at position: 233. Requested branch was: 204.
  Source=System.Private.CoreLib
  StackTrace:
   at System.Reflection.Emit.ILGenerator.BakeByteArray()
   at System.Reflection.Emit.DynamicResolver..ctor(DynamicILGenerator ilGenerator)
   at System.Reflection.Emit.DynamicILGenerator.GetCallableMethod(RuntimeModule module, DynamicMethod dm)
   at System.Reflection.Emit.DynamicMethod.GetMethodDescriptor()
   at System.Reflection.Emit.DynamicMethod.CreateDelegate(Type delegateType)
   at Flee.InternalTypes.Expression`1.Compile(String expression, ExpressionOptions options) in D:\Code\Flee\src\Flee.NetStandard20\InternalTypes\Expression.cs:line 100
   at Flee.InternalTypes.Expression`1..ctor(String expression, ExpressionContext context, Boolean isGeneric) in D:\Code\Flee\src\Flee.NetStandard20\InternalTypes\Expression.cs:line 49
   at Flee.PublicTypes.ExpressionContext.CompileDynamic(String expression) in D:\Code\Flee\src\Flee.NetStandard20\PublicTypes\ExpressionContext.cs:line 198
   at Flee.Console.Program.Main(String[] args) in D:\Code\Flee\test\Flee.Console\Program.cs:line 27

I just changed 2 to 2.1 and it doesn't work. How to solve this?

suriyadi15 commented 3 years ago

This problem has been solved. Change this code error code with solved code. This problem because use IF function, and I replace it with a custom function.

hunkydoryrepair commented 3 years ago

I wonder if this could be the same reason IIS is crashing for some people on 64 bit.
Your function will have worse performance as all the expressions need to be evaluated whereas the built-in only evaluates the needed expresion.

hunkydoryrepair commented 3 years ago

I reproduced this on .Net Framework 4.7.2 without the AND function: "IF(2.1<>2.1,IF(2.1>2.1,2.1,IF(2.1>2.1 AND 2.1<=2.1,2.1,IF(2.1>2.1 AND 2.1<=2.1,2.1,2.1))),IF(2.1>2.1,2.1,IF(2.1>2.1 AND 2.1<=2.1,2.1,IF(2.1>2.1 AND 2.1<=2.1,2.1,2.1))))"

I believe the problem is in BranchManager.ComputeBranches(). This is where the branch addresses are adjusted for long branches vs short branches, but there is an edge case that isn't handled. It seems to only support one depth level of branches going from a short to a long branch. So you if have a long branch that starts inside a short branch, and the extra bytes from that branch cause the short branch to become a long branch, that is handled, But if the short branch that becomes a long ALSO starts within a short branch, that 2nd level short branch also needs to be extended and may also need to become a long branch. But that adjustment is never made.

I'm working on it to see if I can find a fix.

hunkydoryrepair commented 3 years ago

Turns out this is messier than I hoped. ComputeBranches is never called with more than 2 branches, so the possible problems I could see won't manifest.

However, the problem is definitely similar to what I described: nested long branches. Nested conditionals don't generate their correct length until the final time they are compiled.

In order to detect long branches, Flee generates the IL in a temporary mode, will all short branches. But as it breaks the code down it doesn't gather ALL the branches into one array and test them, it on tests at the top of the current level. The offsets are wrong because if there is a long branch inside a conditional expression (a conditional in a conditional in a conditional....) it doesn't do the adjustment for the branches until the 2nd time through, but that time the top level expects the size to be the same, but it isn't so the long branch detection fails.

I have implemented a fix, but it involves outputting NOP in the temp mode generation. This reduces the work of generating code at the lower levels over what exists in the current repo, but it could be better.

Another approach which would track all branches from the very top level, and then fix up all branches and compile only twice the entire expression. Currently, nested expressions can be compiled more than twice. However, since ComputeBranches is only suited for 2 branches, this would require a lot more work and a lot more structural changes maintain a single list of branches. (I think ComputeBranches could work if the branches were sorted in reverse order by start location)

My patch is available in this repo: https://github.com/hunkydoryrepair/Flee

I can submit a pull request unless somebody wants to invest into a better solution.