Crash in js::frontend::TokenStreamSpecific<T>::identifierName
Categories
(Core :: JavaScript Engine, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox61 | --- | wontfix |
firefox62 | --- | wontfix |
firefox63 | --- | wontfix |
firefox64 | --- | wontfix |
firefox65 | --- | wontfix |
firefox66 | --- | wontfix |
firefox67 | --- | wontfix |
People
(Reporter: philipp, Unassigned)
References
Details
(4 keywords)
Crash Data
This bug was filed from the Socorro interface and is report bp-6c136416-7937-48b2-b097-0a6f30180712. ============================================================= Top 10 frames of crashing thread: 0 xul.dll js::frontend::TokenStreamSpecific<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::identifierName js/src/frontend/TokenStream.cpp:1484 1 xul.dll js::frontend::TokenStreamSpecific<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::getTokenInternal js/src/frontend/TokenStream.cpp:1902 2 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::memberExpr js/src/frontend/Parser.cpp:8745 3 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::unaryExpr js/src/frontend/Parser.cpp:8560 4 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::assignExpr js/src/frontend/Parser.cpp:8264 5 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::expr js/src/frontend/Parser.cpp:7930 6 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::expressionStatement js/src/frontend/Parser.cpp:6132 7 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementListItem js/src/frontend/Parser.cpp:7768 8 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementList js/src/frontend/Parser.cpp:4253 9 xul.dll js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionBody js/src/frontend/Parser.cpp:2762 ============================================================= this crash signature is newly showing up in firefox 61 and subsequent builds.
Comment 2•6 years ago
|
||
The top function -- TokenStreamSpecific::identifierName -- is newly extracted from TokenStreamSpecific::getTokenInternal where it had resided for quite some time. The mere presence of crashes inside this function doesn't necessarily prove anything -- they could have just transferred from the one function to the other. Setting that aside, the crash location here has been further altered on trunk -- including so that we know which arm of the prior if-else was followed. bp-9a181ac1-4a44-446f-a46d-3f6650180708 looks like a better report to investigate, and it suggests that the crash is something about creating an atom from source text. Past that, I'm not immediately sure what to investigate next about this, or what can be investigated next. Will think about it more before clearing the needinfo.
Comment 3•6 years ago
|
||
waldo: any results from thinking?
Updated•6 years ago
|
Comment 4•6 years ago
|
||
Good news, maybe? As I look at it now, there are no crashes in 63 or later. The particular change I mentioned in comment 2, that would help to indicate precisely which arm was going awry, was https://hg.mozilla.org/mozilla-central/rev/b64aec5cf563 which that page sez is 63.0a1. So I *think* what's probably happened (unless I fixed the underlying crash, although such was not my intent and I'm not sure how I could have done so!), is that change has moved these crashes into some other function. However -- I did change some mechanics a little, so it's not entirely inconceivable I fixed this somehow. I'm not sure precisely where crashes would migrate after that change -- into drainCharBufferIntoAtom or atomizeSourceChars (note name change because a change after the rev noted above changed that name), maybe. But merely typing either name into the box top right on crash-stats gives no results, which means either I'm misusing the API or there are no such crashes. SO given no crashes in 63, and the two IMO-likeliest places for crashes to migrate have shown no migration, can we call this WORKSFORME now? Will leave it to triage peoples to say if that's really the thing to do here. It is dissatisfying to not know *what* happened here, but ¯\_(ツ)_/¯.
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Still crashing in 63 release and 64 beta
Comment 6•6 years ago
|
||
Waldo, can you take another look at the new crashes?
Updated•6 years ago
|
Updated•5 years ago
|
Comment 7•5 years ago
|
||
Ted took another look here and there's not much we have to go on.
Updated•5 years ago
|
Comment 8•5 years ago
|
||
There are some interesting GeckoView segfaults showing up on 67 such as: https://crash-stats.mozilla.org/report/index/72d88d1b-d1d2-47fe-a7e9-d0dc40190527
There is a bounds check but the type is char16_t which may be signed so seems invalid.
Jeff, as the person familiar with parser and C++ data types can you sanity check that this isn't completely wrong?
Comment 9•5 years ago
|
||
(In reply to Ted Campbell [:tcampbell] from comment #8)
There are some interesting GeckoView segfaults showing up on 67 such as: https://crash-stats.mozilla.org/report/index/72d88d1b-d1d2-47fe-a7e9-d0dc40190527
There is a bounds check but the type is char16_t which may be signed so seems invalid.
"char16_t - type for UTF-16 character representation, required to be large enough to represent any UTF-16 code unit (16 bits). It has the same size, signedness, and alignment as std::uint_least16_t, but is a distinct type. " -- https://en.cppreference.com/w/cpp/language/types
That says it should be unsigned, unless there's a compiler bug.
This smells more like some form of UAF or memory trashing, given the other crashes around here. Note that there are 5-6 crashes this week all from the same install time -- likely the same device, but (as happens) it reported the same crash multiple times.
Comment 10•5 years ago
|
||
char16_t
is definitely required to be unsigned, and we statically assert this.
Whatever the problem is here, that stack doesn't really help us at figuring it out. The true issue could be memory corruption; or it could be aggressive compiler inlining means the bounds-check gets eliminated unexpectedly; or it could be a compiler bug around optimization/inlining; or something else. But there's really nothing to go on so far. :-(
Updated•5 years ago
|
Comment 11•4 years ago
|
||
Removing employee no longer with company from CC list of private bugs.
Updated•2 years ago
|
Updated•1 year ago
|
Comment 12•1 year ago
|
||
Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit auto_nag documentation.
Updated•2 months ago
|
Description
•