1598331 - Crash in [@ OOM | unknown | js::AutoEnterOOMUnsafeRegion::crash | js::irregexp::RegExpParser<T>::ParseDisjunction]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-28112114-ef88-4ebd-98f0-efb380191121.

Seen while looking at release crash stats - this is currently #26 overall on release and didn't have a bug: https://bit.ly/2qyIBG2. Although present in 69, volume seems to have increased in the 70 cycle. I will go back through the correlations to see if I can extract anything useful. Right now there are a few mentions of Facebook in the comments.

Top 10 frames of crashing thread:

0 xul.dll js::AutoEnterOOMUnsafeRegion::crash js/src/vm/JSContext.cpp:1523
1 xul.dll js::irregexp::RegExpParser<char16_t>::ParseDisjunction js/src/irregexp/RegExpParser.cpp
2 xul.dll js::irregexp::ParsePatternSyntax js/src/irregexp/RegExpParser.cpp:2012
3 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::Parser<js::frontend::SyntaxParseHandler, char16_t>::newRegExp js/src/frontend/Parser.cpp:9592
4 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::memberExpr js/src/frontend/Parser.cpp:9156
5 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::unaryExpr js/src/frontend/Parser.cpp:8945
6 xul.dll js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::assignExpr js/src/frontend/Parser.cpp:8589
7 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::argumentList js/src/frontend/Parser.cpp:9027
8 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::memberExpr js/src/frontend/Parser.cpp:9314
9 xul.dll js::frontend::SyntaxParseHandler::Node js::frontend::GeneralParser<js::frontend::SyntaxParseHandler, mozilla::Utf8Unit>::unaryExpr js/src/frontend/Parser.cpp:8945

Comment 1

[Nicolas B. Pierron [:nbp]]

68.56% (556) of users have the same off-thread SyntaxParseTask failing on the helper thread, with the same proto-signature.
This could be used to identify which regex among facebook scripts (mentioned in URL / comments) is causing this crash to happen, and potentially reproduce this issue. (maybe among the Whatsapp scripts)

js::AutoEnterOOMUnsafeRegion::crash
js::irregexp::RegExpParser<T>::ParseDisjunction
js::irregexp::ParsePatternSyntax
js::RegExpObject::create
js::RegExpObject::create<T>
js::frontend::Parser<T>::newRegExp
js::frontend::GeneralParser<T>::memberExpr
js::frontend::GeneralParser<T>::unaryExpr
js::frontend::GeneralParser<T>::assignExpr
js::frontend::GeneralParser<T>::initializerInNameDeclaration
js::frontend::GeneralParser<T>::declarationName
js::frontend::GeneralParser<T>::declarationList
js::frontend::GeneralParser<T>::statementListItem
js::frontend::GeneralParser<T>::statementList
js::frontend::GeneralParser<T>::functionBody
js::frontend::GeneralParser<T>::functionFormalParametersAndBody
js::frontend::GeneralParser<T>::innerFunctionForFunctionBox
js::frontend::Parser<T>::trySyntaxParseInnerFunction
js::frontend::GeneralParser<T>::functionDefinition
js::frontend::GeneralParser<T>::functionExpr
js::frontend::GeneralParser<T>::memberExpr
js::frontend::GeneralParser<T>::unaryExpr
js::frontend::GeneralParser<T>::assignExpr
js::frontend::GeneralParser<T>::expr
js::frontend::GeneralParser<T>::memberExpr
js::frontend::GeneralParser<T>::unaryExpr
js::frontend::GeneralParser<T>::assignExpr
js::frontend::GeneralParser<T>::argumentList
js::frontend::GeneralParser<T>::memberExpr
js::frontend::GeneralParser<T>::unaryExpr
js::frontend::GeneralParser<T>::assignExpr
js::frontend::GeneralParser<T>::expr
js::frontend::GeneralParser<T>::statementListItem
js::frontend::GeneralParser<T>::statementList
js::frontend::Parser<T>::globalBody
js::frontend::ScriptCompiler<T>::compileScript
js::frontend::CompileGlobalScript
ScriptParseTask<T>::parse
js::ParseTask::runTask
js::HelperThread::handleParseWorkload

Iain, could you look at this crash signature and at best identify the faulty regexp to add it to our test case?
Yoric has a firefox addon for scrapping website, which might be handy.

Comment 2

[Iain Ireland [:iain]]

Bug 1597119 is a duplicate of this.

Looking at crashstats, it appears to be an external change. The regexp code in question has not been changed in years, and the spike started Nov 16, two weeks after the release of 70.0.1. On 70.0.1, 90% of the crashes are 32-bit (compared to 40% in an equivalent period a month ago). The data seems consistent with the hypothesis that somebody (Facebook?) pushed a change on the 16th that is creating a ridiculously big regexp, which is causing us to OOM on 32-bit Windows machines.

Ted and I are going to try browsing Facebook and dumping regexes to see if anything shows up.

Comment 3

[Pascal Chevrel:pascalc (PTO until April 26)]

We have shipped our last beta for 71, I am keeping it as fix-optional for 71 in case we find a solution that could be included in a dot release.

Comment 4

[Julien Cristau [:jcristau]]

(In reply to Iain Ireland [:iain] from comment #2)

Ted and I are going to try browsing Facebook and dumping regexes to see if anything shows up.

Did that turn up anything?

Comment 5

[Iain Ireland [:iain]]

Sorry, we haven't gotten around to it yet.

Based on the graph, it looks like there was a spike of crashes here, which went down on its own, then a second spike later on, which also went away. I think that's consistent with the hypothesis that we see a spike when some top site deploys a giant regexp until they fix it.

Although now that I think about it , there's a chance that this might be related to bug 1603115. I'll ask in that bug to see if the dates line up.

Comment 6

[:Gijs (he/him)]

There was another small tick when updates from bug 1604655 went live, so it seems plausible that the big spikes were indeed caused by the blocklist regexes. The patch in bug 1603115 will further address these uses of regex, but at least bug 1604655 made sure that we won't be hitting the same crashes in quite that volume again for further blocklist updates.

I haven't looked, but it may be worth checking for other/further blocklist updates (can check with hg log for blocklist.xml) around the november 16 mark which might explain the spike then.

Comment 7

[Steven DeTar [:sdetar]]

Iain, any updates on this bug? Is this bug actionable and still a P1? Will this be addressed with your planned Regexp changes?

Comment 8

[Iain Ireland [:iain]]

Looking at the crash signature, it seems that this problem has gone away. This might have been at least partially related to the blocklist changes in bug 1604655 and co, but either way it seems to have resolved itself. I don't think there's anything actionable here. Resolving as fixed.