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

Test262 panics #817

Closed
Razican opened this issue Oct 8, 2020 · 3 comments · Fixed by #1285
Closed

Test262 panics #817

Razican opened this issue Oct 8, 2020 · 3 comments · Fixed by #1285
Labels
bug Something isn't working good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com
Milestone

Comments

@Razican
Copy link
Member

Razican commented Oct 8, 2020

This is a full list of panics the current ECMAScript test suite (test262) gives, 13-14 in total. We should aim for 0 panics (even before we try to implement the rest of the spec):

  • Object already borrowed: BorrowMutError (#1059), count: 2 Example:
    thread 'main' panicked at 'Object already borrowed: BorrowMutError', boa/src/object/internal_methods.rs:198:18
  • Identifier arguments has already been declared (#1062), count: 8 Example:
    thread 'main' panicked at 'Identifier arguments has already been declared', boa/src/environment/declarative_environment_record.rs:52:13
  • range start index 2 out of range for slice of length 0 (#1241), count: 2 Example:
    thread 'main' panicked at 'range start index 2 out of range for slice of length 0', boa/src/builtins/function/mod.rs:123:45
  • Could not get slice (#1242), count: 1 Example:
    thread 'main' panicked at 'Could not get slice', boa/src/builtins/regexp/mod.rs:413:56

There is another random panic that seems to not always occur, so we don't have too much information about it, or the exact test where it happens (there are multiple tests with that name):

thread 'main' panicked at 'Object already mutably borrowed: BorrowError', boa/src/value/type.rs:50:27
last panic was on test "returns-iterator"

It seems to happen here when calling the get_type() function in an object, when checking if the object is a function. This happens if the object is already mutably borrowed, and this borrow will fail. Interestingly, this doesn't happen in every run.

How to replicate / find code to reproduce:

If you run the test262 suite, you will be able to see for each of the panics which one was the panicking test. To run it:

cargo run --release --bin boa_tester -- run -v 2>error.log

This will put all panics and errors in the error.log file. For each panic, you will see something like this:

thread 'main' panicked at 'Could not get slice', boa/src/builtins/regexp/mod.rs:413:56
last panic was on test "failure-g-lastindex-reset"

This means that the panic was on that test file. You can search for that test file here: https://github.com/tc39/test262/find/main

@Razican Razican added bug Something isn't working good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com labels Oct 8, 2020
@Razican Razican added this to the v0.11.0 milestone Oct 8, 2020
@Razican Razican mentioned this issue Oct 8, 2020
@Razican
Copy link
Member Author

Razican commented Oct 21, 2020

Updated the list of panics, counting those that are still present even after #906 gets merged.

The two main areas I see for concern are the Array implementation, whose methods seem not to be spec compliant and seems to use too many expects or unwraps, and the RegExp implementation.

Then, we should re-check the list and make sure that all the environment related panics get solved in #659.

@Razican
Copy link
Member Author

Razican commented Jan 1, 2021

I have updated the list of panics after the merges of the latest changes. It seems that our current "big thing" is RegExp, along with a couple of places in environment variable definitions and retrievals.

Razican added a commit that referenced this issue Jan 7, 2021
This also implements a spec-compliant `parseInt()` function.
Razican added a commit that referenced this issue Jan 8, 2021
* Fixed a bunch of test262 panics (#817)

This also implements a spec-compliant `parseInt()` function.

* Reverted a test

* Addressed long comments from review
@Razican Razican modified the milestones: v0.11.0, v0.12.0 Jan 11, 2021
@Razican
Copy link
Member Author

Razican commented Jan 12, 2021

I have updated the list of panics. We now have one issue per panic (except for the one that happens randomly). This should help tackle the remaining Test262 panics on the Boa 0.12 release, and make it much safer to use in other projects.

@Razican Razican modified the milestones: v0.12.0, v0.13.0 May 22, 2021
@0x7D2B 0x7D2B linked a pull request May 27, 2021 that will close this issue
@0x7D2B 0x7D2B modified the milestones: v0.13.0, v0.12.0 May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants