Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boolean expressions with parenthesis do not represent themselves properly in decompiler output. #41

Open
AngeloD2022 opened this issue Aug 31, 2022 · 8 comments · May be fixed by #43
Open
Labels
bug Something isn't working

Comments

@AngeloD2022
Copy link
Owner

For example:

ESTK Input:

(a || b && c) || !d

Decompiler output given the above compiled input:

((((a) || (b)) && (c)) || (!d))

which can be simplified to:

((a || b) && c) || ! d

Problems

  1. These two expressions are not algebraically equivalent.
  2. There are many redundant parenthesis.
@AngeloD2022 AngeloD2022 added the bug Something isn't working label Aug 31, 2022
@psyirius
Copy link
Collaborator

Need someway of backtracking when dumping the ast as string.
It's can be achieved using some contextual parsing and dumping.

@AngeloD2022 AngeloD2022 linked a pull request Aug 31, 2022 that will close this issue
@AngeloD2022
Copy link
Owner Author

Need someway of backtracking when dumping the ast as string. It's can be achieved using some contextual parsing and dumping.

I added code to #43.
See what you think. I tested several different parenthesis configurations. It appears that Adobe's JavaScript interpreter parses logical expressions oddly. I'll expand on this later.

@psyirius
Copy link
Collaborator

psyirius commented Sep 8, 2022

I did checked out some and wasn't satisfied about the results.
gotta run some real test-cases to confirms.
not having much time to focus on FOSS.

@AngeloD2022
Copy link
Owner Author

I studied the parsing of logical expressions with ExtendScript by making a script that generates a truth table and this was my conclusion...

CleanShot 2022-09-08 at 14 08 35@2x

@sfeigl
Copy link

sfeigl commented Sep 19, 2022

Adobe ExtendScript seems not to honor operator precedence correctly for OR and AND.

Compiling

((((a) || (b)) && (c)) || (!d))

to jsxbin results in the same binary representation than

(a || b && c) || !d

Expecting standard algebraic precedence one would expect that these expressions are not the same. But in ExtendScript they are. You have to use

(a || (b && c)) || !d

to force a higher precedence for && in ExtendScript.

There are similar problems with bitwise operators

The decompilation process is correct - with some unneccessary parenthesis.

@AngeloD2022
Copy link
Owner Author

AngeloD2022 commented Sep 19, 2022

The decompilation process is correct - with some unnecessary parenthesis.

@sfeigl I deliberately added some unnecessary parenthesis to make any decompiled LogicalExpression evaluate equivalently in more modern javascript engines1 (but it also makes the output more intuitive in my opinion). I tested it by making a script that generates a truth table and analyzing the output of different parenthesis configurations. I then implemented the parenthesis decompiler logic such that it may function equivalently on both ExtendScript and V8 runtimes.

Footnotes

  1. This is with the assumption that you did your analysis of the output with the linked pull request (Fix boolean parenthesis problem. (Tested empirically) #43), containing more refined parenthesis decompilation logic.

@AngeloD2022
Copy link
Owner Author

function ttbl(condition) {
    var table = "";
    for(var a = 0; a < 2; a++) {
        for(var b = 0; b < 2; b++) {
            for(var c = 0; c < 2; c++) {
                for(var d = 0; d < 2; d++) {
                    table += a + " " + b + " " + c + " " + d + " = " + condition(a,b,c,d) + "\n";
                }
            }
        }
    }
    alert(table);
}

// decompiler input before encoding to jsxbin
ttbl(function(a, b, c, d) {return a || b && c || d})

// JSXBIN encoded...
// @JSXBIN@[email protected]@MyBbyBn0ABJAnAEjzEjUjUjCjMBfRBNyBnAMAbyBn0ABZAnAUzCjcjcCUzChGhGDUCVzBjB
// EfAVzBjCFfBnnVzBjDGfCnnVzBjEHfDnnAEE40BhAF4B0AhAG4C0AhAH4D0AhAE0AzAICAff0DIByB

// the improved decompiler output... (truth table in V8 is now equivalent to that of the ExtendScript runtime, but 
// the new expression is also functionally equivalent to the old one when ran in ES)
ttbl(function(a,b,c,d) {return (a || b) && c || d})

// The decompiler now produces output that forces order of operations equivalent to ES with modern interpreters like V8,
// and it manages to do it such that it does not affect the order of operations if evaluated in the ES runtime. Awesome! :)

@AngeloD2022
Copy link
Owner Author

@psyirius Circling back to this issue. What did you think of the results?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants