Closed Bug 1262936 Opened 8 years ago Closed 8 years ago

Crash [@ js::irregexp::ExecuteCode] with OOM

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, Whiteboard: [jsbugmon:update,bisect])

Crash Data

Attachments

(1 file)

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.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
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)
Group: javascript-core-security
Attachment #8739326 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/9943bdffe504
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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)
jandem, I meant comment 5 as a question for you.
Flags: needinfo?(jdemooij)
(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)
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+
(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.
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.