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

Parser Panic #240

Closed
adumbidiot opened this issue Feb 2, 2020 · 10 comments · Fixed by #567
Closed

Parser Panic #240

adumbidiot opened this issue Feb 2, 2020 · 10 comments · Fixed by #567
Labels
bug Something isn't working help wanted Extra attention is needed parser Issues surrounding the parser
Milestone

Comments

@adumbidiot
Copy link
Contributor

When putting the following in test.js:
var`a = 1;
running the command cargo run -- test.js will give the following panic:

    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target\debug\boa.exe test.js`
thread 'main' panicked at '1:4: Unexpected '`'', src/lib\syntax\lexer.rs:620:22
stack backtrace:
   0:     0x7ff636d9a689 - backtrace::backtrace::trace_unsynchronized
                               at C:\Users\VssAdministrator\.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.40\src\backtrace\mod.rs:66
   1:     0x7ff636d9a689 - std::sys_common::backtrace::_print_fmt
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\sys_common\backtrace.rs:77
   2:     0x7ff636d9a689 - std::sys_common::backtrace::_print::{{impl}}::fmt
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\sys_common\backtrace.rs:59
   3:     0x7ff636db007b - core::fmt::write
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libcore\fmt\mod.rs:1052
   4:     0x7ff636d98244 - std::io::Write::write_fmt<std::sys::windows::stdio::Stderr>
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\io\mod.rs:1428
   5:     0x7ff636d9cfbc - std::sys_common::backtrace::_print
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\sys_common\backtrace.rs:62
   6:     0x7ff636d9cfbc - std::sys_common::backtrace::print
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\sys_common\backtrace.rs:49
   7:     0x7ff636d9cfbc - std::panicking::default_hook::{{closure}}
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:204
   8:     0x7ff636d9cc0c - std::panicking::default_hook
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:224
   9:     0x7ff636d9d70f - std::panicking::rust_panic_with_hook
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:472
  10:     0x7ff636d9d295 - std::panicking::begin_panic_handler
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:380
  11:     0x7ff636d9d20c - std::panicking::begin_panic_fmt
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:334
  12:     0x7ff6369f4511 - boa::syntax::lexer::Lexer::lex
                               at C:\Users\nathaniel\Documents\GitHub\boa\src\lib\syntax\lexer.rs:620
  13:     0x7ff6369f3121 - boa::parser_expr
                               at C:\Users\nathaniel\Documents\GitHub\boa\src\lib\lib.rs:24
  14:     0x7ff6369f32b8 - boa::forward
                               at C:\Users\nathaniel\Documents\GitHub\boa\src\lib\lib.rs:33
  15:     0x7ff6369f36b8 - boa::exec
                               at C:\Users\nathaniel\Documents\GitHub\boa\src\lib\lib.rs:56
  16:     0x7ff6369f1d7d - boa::main
                               at C:\Users\nathaniel\Documents\GitHub\boa\src\bin\bin.rs:38
  17:     0x7ff6369f2335 - std::rt::lang_start::{{closure}}<core::result::Result<(), std::io::error::Error>>
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\src\libstd\rt.rs:67
  18:     0x7ff636d9d137 - std::rt::lang_start_internal::{{closure}}
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\rt.rs:52
  19:     0x7ff636d9d137 - std::panicking::try::do_call<closure-0,i32>
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:305
  20:     0x7ff636da06a2 - panic_unwind::__rust_maybe_catch_panic
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libpanic_unwind\lib.rs:86
  21:     0x7ff636d9d932 - std::panicking::try
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panicking.rs:281
  22:     0x7ff636d9d932 - std::panic::catch_unwind
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\panic.rs:394
  23:     0x7ff636d9d932 - std::rt::lang_start_internal
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\/src\libstd\rt.rs:51
  24:     0x7ff6369f230b - std::rt::lang_start<core::result::Result<(), std::io::error::Error>>
                               at /rustc/212b2c7da87f3086af535b33a9ca6b5242f2d5a7\src\libstd\rt.rs:67
  25:     0x7ff6369f2100 - main
  26:     0x7ff636db6878 - invoke_main
                               at d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  27:     0x7ff636db6878 - __scrt_common_main_seh
                               at d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  28:     0x7fff816a7bd4 - BaseThreadInitThunk
  29:     0x7fff82acced1 - RtlUserThreadStart
error: process didn't exit successfully: `target\debug\boa.exe test.js` (exit code: 101)

This should probably be an error, not a panic.

@jasonwilliams
Copy link
Member

Thanks @adumbidiot
You’ve touched upon on a deeper problem Boa has right now, which is lack of error handling on all layers. The best way for us to get up and running was to just let things panic, but we should put some work in to have proper stderr messages, maybe starting with the lexer/parser.

@jasonwilliams jasonwilliams added bug Something isn't working help wanted Extra attention is needed labels Feb 2, 2020
@adumbidiot
Copy link
Contributor Author

Would it be helpful to try to replace some of the panics in the lexer with the LexerErrors?

@croraf
Copy link
Contributor

croraf commented Feb 2, 2020

@jasonwilliams I guess a LexerError is when the user inputs something that is not allowed as per JS specification. That will show him that his code is invalid. What happens here is that ` is not implemented in Boa, so I think "panic" is ok, as it is actually bugs out in Boa before realizing it is invalid as per JS syntax.

If template literals were implemented it should show LexerError, something like:
image

image

@adumbidiot
Copy link
Contributor Author

I still kinda feel like the engine should still return an error. Just as older engines reject newer syntax with lexing and parsing errors and such, I feel like Boa should reject syntax it doesn't support with errors.

@jasonwilliams
Copy link
Member

jasonwilliams commented Feb 2, 2020

It should be possible to return an Err(LexerError) here:
https://github.com/jasonwilliams/boa/blob/master/src/lib/syntax/lexer.rs#L620-L622

Then perform a match here:
https://github.com/jasonwilliams/boa/blob/master/src/lib/lib.rs#L24

printing a stderr with the message from the Err() if there is one.
This would be a good start.

@jasonwilliams
Copy link
Member

This has now been fixed, thanks @adumbidiot

@adumbidiot
Copy link
Contributor Author

I feel like a lot more work has to be done here; My pull only fixed some of the lexer panics and none of the parser panics.

@jasonwilliams jasonwilliams reopened this Feb 4, 2020
@jasonwilliams jasonwilliams added the parser Issues surrounding the parser label Mar 31, 2020
@jasonwilliams
Copy link
Member

@adumbidiot have you tried this against the new parser?

jasonwilliams added a commit that referenced this issue Mar 31, 2020
New parser!
Plus loads of tidy up in various places.

Co-authored-by: Jason Williams <jwilliams720@bloomberg.net>
Co-authored-by: HalidOdat <halidodat@gmail.com>
Co-authored-by: Iban Eguia <iban.eguia@cern.ch>
Co-authored-by: Iban Eguia <razican@protonmail.ch>
@jasonwilliams
Copy link
Member

I'm going to close this issue as the main problem in question has been fixed, and some other parser issues have been resolved.

If there are new issues we should make a ticket for them

@Razican Razican linked a pull request Sep 2, 2020 that will close this issue
@Razican Razican added this to the v0.10.0 milestone Sep 2, 2020
@Razican
Copy link
Member

Razican commented Sep 2, 2020

I want to re-open this, since it seems this is now also panicking. We should never panic, and checking what this gives in Firefox, for example, I can see the following:

Uncaught SyntaxError: `` literal not terminated before end of script

So, let's re-open it and track it, since we get this panic:

I/O error: Unterminated template literal
thread 'main' panicked at 'explicit panic', boa/src/lib.rs:94:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

It seems that in the test262 branch this is fixed:

"SyntaxError": "I/O error: Unterminated template literal"

@Razican Razican reopened this Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed parser Issues surrounding the parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants