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

Context: Expose instruction count #1

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

Conversation

sam-finch-tezos
Copy link
Collaborator

Exposes the instruction counter and fix a bug allowing unreachable behaviour.

  • If the "fuzz" feature is enabled the instructions_remaining counter is a public property.
  • The NoInstructionsRemaining error can now be converted into a javascript error, preventing boa from panicking if the error is thrown async

@johnyob johnyob changed the title Sam.finch@/context/expose instruction count Context: Expose instruction count Oct 27, 2023
@johnyob johnyob changed the base branch from ajob410@create-realm-with-default-globals to main October 27, 2023 14:17
@johnyob johnyob force-pushed the sam.finch@/context/expose-instruction-count branch 4 times, most recently from f8a1509 to 01ae4c4 Compare October 27, 2023 14:30
Copy link
Collaborator

@johnyob johnyob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnyob johnyob force-pushed the sam.finch@/context/expose-instruction-count branch from 53e6ecb to 6941e76 Compare November 16, 2023 21:09
Comment on lines 943 to 822
unreachable!(
"The NoInstructionsRemain native error cannot be converted to an opaque type."
)
(constructors.eval_error().prototype(), ErrorKind::Eval)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to convert the native error into an opaque one? My concern would be that this error could be caught using try-catch

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the error is thrown async then boa will convert it into an opaque error when it rejects the promise. This causes a panic. If an out of gas error is caught it will immediately be rethrown because you're still out of gas.

@sam-finch-tezos sam-finch-tezos force-pushed the sam.finch@/context/expose-instruction-count branch 2 times, most recently from d68e3fc to 4da6387 Compare November 29, 2023 13:19
@sam-finch-tezos sam-finch-tezos force-pushed the sam.finch@/context/expose-instruction-count branch from b8c11dc to 41b24d5 Compare December 13, 2023 11:44
@johnyob johnyob force-pushed the sam.finch@/context/expose-instruction-count branch from 41b24d5 to 533f6a5 Compare October 7, 2024 21:31
Copy link

github-actions bot commented Oct 7, 2024

Test262 conformance changes

Test result main count PR count difference
Total 48,494 48,494 0
Passed 43,609 43,609 0
Ignored 1,500 1,500 0
Failed 3,385 3,385 0
Panics 0 0 0
Conformance 89.93% 89.93% 0.00%

@johnyob johnyob force-pushed the sam.finch@/context/expose-instruction-count branch 2 times, most recently from 42c4851 to 5ba29ab Compare October 7, 2024 22:03
@johnyob johnyob force-pushed the sam.finch@/context/expose-instruction-count branch from 5ba29ab to e5f312a Compare October 8, 2024 17:49
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