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

feature/void operator #465

Closed
wants to merge 1 commit into from
Closed

Conversation

Yi2255
Copy link
Contributor

@Yi2255 Yi2255 commented Nov 14, 2024

Add void operator feature. The specification ref MDN

@saelo
Copy link
Collaborator

saelo commented Nov 15, 2024

Thanks! So I think this isn't quite the right way to do this. For one, we generally need the output of lifting to be deterministic (i.e. the same FuzzIL program lifted twice must result in the same output text), and this breaks that (and you can see various tests failing due to that here: https://github.com/googleprojectzero/fuzzilli/actions/runs/11836407403/job/33049141843?pr=465). This is bad because now the same program might behave differently between executions and so cause unreliability. Also, this way only a few selected operations can be prefixed with void, while in theory, every expression can use it. I think instead the right way to do this would either be to add another unary operator (should be fairly easy) or another JSOperation (a bit more work, but should be fairly straightforward). A reason to go with a new JSOperation might be that this operation is probably fairly uninteresting (compared with the other unary operators) as it basically just means the engine will throw away the return value. So maybe that would be the better approach? WDYT?

@Yi2255
Copy link
Contributor Author

Yi2255 commented Nov 18, 2024

You are right, all expressions can use void. Using unary is a very clever way to do it. Thanks a lot for the correction!

@Yi2255 Yi2255 closed this Nov 18, 2024
@Yi2255 Yi2255 deleted the feature-void branch November 18, 2024 23:31
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.

2 participants