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

Missing callback for unary in conditional #98

Open
BarrensZeppelin opened this issue Oct 26, 2021 · 2 comments
Open

Missing callback for unary in conditional #98

BarrensZeppelin opened this issue Oct 26, 2021 · 2 comments

Comments

@BarrensZeppelin
Copy link
Contributor

Hello,

I have the following sample Jalangi2 analysis:

((sandbox) => {
	function MyAnalysis() {
		this.unary = function(iid, op, left) {
			console.log('unary', op, left, arguments);
		};

		this.conditional = function(iid, cond) {
			console.log('cond', arguments);
		};
	}

	sandbox.analysis = (new MyAnalysis());
})(J$)

And test program:

function foo(input) {
	!input;
	if(!input) {
		return 'oh no';
	}
};

foo();

When I run the analysis with Jalangi2 I get the expected output:

unary ! undefined [Arguments] { '0': 10, '1': '!', '2': undefined, '3': true }
unary ! undefined [Arguments] { '0': 18, '1': '!', '2': undefined, '3': true }
cond [Arguments] { '0': 8, '1': true }

But when I run it with NodeProf.js I get this output:

unary ! undefined [Arguments] { '0': 3, '1': '!', '2': undefined, '3': true }
cond [Arguments] { '0': 4, '1': undefined }

It misses a callback for unary inside the conditional, and the result argument of conditional is undefined instead of true.

@alexjordan
Copy link
Collaborator

Hi @BarrensZeppelin,

I think what happens here is due to graal-js optimizing away the unary operation for simple conditions. If you change the condition to e.g. if(!!input) the unary callback will be invoked.

These optimizations (or simplifications) happen when the AST is built and generally cannot be disabled, maybe @eleinadani knows whether the current behavior exposed to NodeProf is correct or not (missing materialization of nodes).

@dervirvel
Copy link

Hi, just noticed the same issue. Is there any walkaround for it?

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

No branches or pull requests

3 participants