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

Fix SyntaxError due to function declarations #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christofferqa
Copy link
Contributor

See #107.

@msridhar
Copy link
Contributor

@christofferqa can you also add a regression test for this issue?

@christofferqa
Copy link
Contributor Author

@msridhar Added a test case that fails with the current master branch:

christofferqa:jalangi2 cqa$ python scripts/test.analysis.py 
es2015_syntax failed

analysis exception!!!
/jalangi2/tests/unit/es2015_syntax_jalangi_.js:23
            var f = J$.X1(41, J$.W(33, 'f', J$.T(25, 42, 22, false), f, 3));
                                                                          ^
SyntaxError: Identifier 'f' has already been declared
    at Object.exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:513:28)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Function.Module.runMain (module.js:575:10)
    at startProgram (/jalangi2/src/js/commands/direct.js:70:13)
    at MyAnalysis.onReady (/jalangi2/src/js/runtime/analysisCallbackTemplate.js:638:13)
    at ChainedAnalyses.self.(anonymous function) [as onReady] (/jalangi2/src/js/sample_analyses/ChainedAnalyses.js:65:59)

@ksen007
Copy link
Contributor

ksen007 commented Nov 23, 2016

what is es2015?

@christofferqa
Copy link
Contributor Author

ECMAScript 2015. I added the test case to show that the current version of Jalangi throws a SyntaxError for code that is not syntactically correct itself (the instrumented program is, though).

I have come across this issue after using Jalangi to instrument some of the most popular libraries for web applications.

@ksen007
Copy link
Contributor

ksen007 commented Nov 23, 2016

Jalangi only supports ES5. It cannot handle any later version.

On Nov 22, 2016, at 11:44 PM, christofferqa notifications@github.com wrote:

ECMAScript 2015. I added the test case to show that the current version of Jalangi throws a SyntaxError for code that is not syntactically correct itself (the instrumented program is, though).

I have come across this issue after using Jalangi to instrument some of the most popular libraries for web applications.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub #109 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AAw0Ob-7gilmZEH4VYtdsrKUxsG0YKNlks5rA-7kgaJpZM4J8yBv.

@msridhar
Copy link
Contributor

@ksen007 reading #107 I think the issue is that Jalangi-instrumented code that might have worked in previous browser versions no longer works, due to changes in the browsers related to ES2015. @christofferqa is that right?

@christofferqa
Copy link
Contributor Author

@msridhar Yes, that is the case. Since the browsers are now implementing ES2015, Jalangi introduces a SyntaxError on certain code.
@ksen007 This is not a matter of extending Jalangi to handle new syntactic JavaScript constructs -- it is merely a matter of adjusting the instrumentation such that it does not introduce a SyntaxError.

Try to instrument this simple program using Jalangi, and run the instrumented program in Chrome:

function inc() {}
var inc = function (i) {
  return i+1;
};

@floledermann
Copy link

floledermann commented Jul 5, 2019

This patch breaks functions that redefine themselves. Simple test case:

function _test() {
  _test = function() {
    return "A";
  }
  return _test();
}
var result = _test();

With your patch applied, this sends Jalangi off into an infinite loop (since _test is never redefined)

While this may seem like an obscure testcase, is is perfectly valid syntax and unfortunately used e.g. by babel to polyfill the typeof operator.

@floledermann
Copy link

This is really weird and I do not fully understand it, but it seems that modern JS engines have trouble redefining named functions. I am not set up to provide a formal patch to your PR, but if I remove assigning a name to the generated function (var _test = function **_test** { ... }) (lines 1829 and 1832 in your patch of esnstrument.js) then the testcase given above runs correctly after instrumentation.

I guess assigning a name to the function literal is not really necessary, at least my tests all run correctly after the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants