Closed
Bug 1262936
Opened 8 years ago
Closed 8 years ago
Crash [@ js::irregexp::ExecuteCode] with OOM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: decoder, Assigned: jandem)
References
(Blocks 1 open bug)
Details
(Keywords: crash, testcase, Whiteboard: [jsbugmon:update,bisect])
Crash Data
Attachments
(1 file)
722 bytes,
patch
|
bhackett1024
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 3930bfe289c8 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-simulator=arm --enable-debug, run with --fuzzing-safe): function testRegexp(res, mode, strings) { re = RegExp(res) try { for (i = 0; i < 1; ++i) str = strings[0] print("//" + str.split()) print(function() {}) print("//" + str.replace(re, function() {})) } catch (e) {} } testRegexp("\\(a\\(b\\)*b\\2", 'gi', [""]); oomTest(function() { testRegexp("\\(a\\(b\\)*b\\2", 'gi', ["abcdababaBcDfoobar!bas"]); testRegexp("a*?X(.?){4,}a\\(a\\*b\\2", 'gi', ["X1234567YabcdababaBcDfoo!bar!bas"]); }) Backtrace: Program received signal SIGSEGV, Segmentation fault. writeW (this=0xf7a1c000, instr=0xf7fd1c9c, value=-134406588, addr=0) at js/src/jit/arm/Simulator-arm.cpp:1543 #0 writeW (this=0xf7a1c000, instr=0xf7fd1c9c, value=-134406588, addr=0) at js/src/jit/arm/Simulator-arm.cpp:1543 #1 js::jit::Simulator::decodeType2 (this=0xf7a1c000, instr=0xf7fd1c9c) at js/src/jit/arm/Simulator-arm.cpp:3201 #2 0x084fb455 in js::jit::Simulator::instructionDecode (this=this@entry=0xf7a1c000, instr=instr@entry=0xf7fd1c9c) at js/src/jit/arm/Simulator-arm.cpp:4409 #3 0x084ff364 in execute<false> (this=0xf7a1c000) at js/src/jit/arm/Simulator-arm.cpp:4479 #4 js::jit::Simulator::callInternal (this=this@entry=0xf7a1c000, entry=entry@entry=0xf7fd1c90 "m") at js/src/jit/arm/Simulator-arm.cpp:4567 #5 0x084ff890 in js::jit::Simulator::call (this=0xf7a1c000, entry=entry@entry=0xf7fd1c90 "m", argument_count=argument_count@entry=1) at js/src/jit/arm/Simulator-arm.cpp:4650 #6 0x088e8824 in js::irregexp::ExecuteCode<unsigned char> (cx=cx@entry=0xf7a76020, codeBlock=codeBlock@entry=0xf436d010, chars=0xf43635e8 "abcdababaBcDfoobar!bas", start=start@entry=0, length=length@entry=22, matches=matches@entry=0xffffb2f0, endIndex=endIndex@entry=0x0) at js/src/irregexp/RegExpEngine.cpp:1899 #7 0x0873ca4f in js::RegExpShared::execute (this=this@entry=0xf7a911d0, cx=cx@entry=0xf7a76020, input=input@entry=..., start=start@entry=0, sticky=false, matches=matches@entry=0xffffb2f0, endIndex=endIndex@entry=0x0) at js/src/vm/RegExpObject.cpp:648 #8 0x085f8f41 in DoMatchForReplaceLocal (rightContextOffset=<synthetic pointer>, rdata=..., re=..., linearStr=..., res=0xf7ac7d30, cx=0xf7a76020) at js/src/jsstr.cpp:2649 #9 StrReplaceRegExp (cx=0xf7a76020, rdata=...) at js/src/jsstr.cpp:3388 #10 0x085f9b79 in str_replace_regexp (rdata=..., args=..., cx=0xf7a76020) at js/src/jsstr.cpp:3411 #11 js::str_replace (cx=cx@entry=0xf7a76020, argc=argc@entry=2, vp=vp@entry=0xf45ffe80) at js/src/jsstr.cpp:3755 #12 0x084fcfe3 in js::jit::Simulator::softwareInterrupt (this=0xf7a1c000, instr=0xf41f45b4) at js/src/jit/arm/Simulator-arm.cpp:2359 [...] #24 0x085167d0 in JS_CallFunction (cx=cx@entry=0xf7a76020, obj=..., fun=fun@entry=..., args=..., rval=rval@entry=...) at js/src/jsapi.cpp:2865 #25 0x08837fdb in OOMTest (cx=0xf7a76020, argc=1, vp=0xf4123058) at js/src/builtin/TestingFunctions.cpp:1311 [...] #38 main (argc=3, argv=0xffffccf4, envp=0xffffcd04) at js/src/shell/js.cpp:7443 eax 0xf7fd1e44 -134406588 ebx 0x9891f38 159981368 ecx 0x0 0 edx 0x4 4 esi 0xf7fd1e44 -134406588 edi 0x4 4 ebp 0xffffafc8 4294946760 esp 0xffffafa0 4294946720 eip 0x84effb3 <js::jit::Simulator::decodeType2(js::jit::SimInstruction*)+179> => 0x84effb3 <js::jit::Simulator::decodeType2(js::jit::SimInstruction*)+179>: mov %esi,(%ecx) 0x84effb5 <js::jit::Simulator::decodeType2(js::jit::SimInstruction*)+181>: add $0x1c,%esp Marking s-s because this is a JIT crash. I've only seen crashes at 0x0, so it might be a safe crash but I'll let the JS developers decide that. According to Socorro, this is a frequent crash on ARM that is indistinguishable from other crashes in irregexp and probably accounts for the vast majority of all crashes in that area.
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 1•8 years ago
|
||
In RegExpStack::reset we call js_realloc to shrink the stack and this can return nullptr on failure. With this patch we just keep the old stack size when that happens. This is a safe near-null crash. The test didn't repro on tip so it's probably not very useful to add.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8739326 -
Flags: review?(bhackett1024)
Assignee | ||
Updated•8 years ago
|
Group: javascript-core-security
Updated•8 years ago
|
Attachment #8739326 -
Flags: review?(bhackett1024) → review+
Comment 3•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9943bdffe504
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 5•8 years ago
|
||
Did this affect 46 and 47? If bug 1264612 is a duplicate, that seems likely. And in that case this might be upliftable to 47 (too late for 46 though)
status-firefox46:
--- → wontfix
status-firefox47:
--- → affected
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5) > Did this affect 46 and 47? If bug 1264612 is a duplicate, that seems likely. > And in that case this might be upliftable to 47 (too late for 46 though) Not sure if uplifting is necessary or worth it but we can. I'll file the request for 47.
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8739326 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: irregexp import, older bug. [User impact if declined]: Crashes on OOM. [Describe test coverage new/current, TreeHerder]: Fixes the fuzz test, on m-c for a while. [Risks and why]: Very low, just adds an OOM check. [String/UUID change made/needed]: None.
Attachment #8739326 -
Flags: approval-mozilla-aurora?
Comment on attachment 8739326 [details] [diff] [review] Patch OOM crash fix, Aurora47+
Attachment #8739326 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/239bda8cda35
Comment 11•8 years ago
|
||
hi, i'm not sure if the patch was supposed to address crashes with that signature in general - in 47 beta builds certainly a few of that kind remain: https://crash-stats.mozilla.com/search/?signature=%3Djs%3A%3Airregexp%3A%3AExecuteCode%3CT%3E&version=47.0b&_facets=signature&_facets=platform_pretty_version&_facets=version&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to philipp from comment #11) > hi, i'm not sure if the patch was supposed to address crashes with that > signature in general It wasn't. This OOM bug is not super likely to occur in practice.
Comment 13•8 years ago
|
||
My partner hit this tonight on his Galaxy S6 Edge with Fennec 48.0: https://crash-stats.mozilla.com/report/index/48fc30dd-6645-4df1-9bc8-76e292160902
You need to log in
before you can comment on or make changes to this bug.
Description
•