Missing OOM handling in js::ParseRegExpFlags

RESOLVED FIXED in Firefox 63

Status

()

P3
normal
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: anba, Assigned: khyperia)

Tracking

(Blocks: 1 bug)

Trunk
mozilla63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 months ago
Missing OOM handling here [1]. I'm not sure why we don't simply pass |cx| to JS::CharsToNewUTF8CharsZ to properly propagate the OOM error?

[1] https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/js/src/vm/RegExpObject.cpp#1424-1426


Test case:
---
oomTest(function() {
    for (var i = 0; i < 10; ++i) {
        try {
            RegExp("", "gimuyz");
        } catch {}
    }
});
---
Priority: -- → P3
(Assignee)

Comment 1

4 months ago
Created attachment 8999973 [details] [diff] [review]
OOM handling in RegExp construction
(Assignee)

Updated

4 months ago
Assignee: nobody → khyperia
Status: NEW → ASSIGNED
(Assignee)

Updated

4 months ago
Attachment #8999973 - Flags: review?(jorendorff)
Comment on attachment 8999973 [details] [diff] [review]
OOM handling in RegExp construction

Review of attachment 8999973 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Thanks!
Attachment #8999973 - Flags: review?(jorendorff) → review+
(Assignee)

Updated

4 months ago
Keywords: checkin-needed

Comment 4

4 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e58207e220a
Add OOM handling in RegExp construction. r=jorendorff
Keywords: checkin-needed
Backed out for spidermonkey failures at oom-in-construction.js

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/083414a62ea15be84270efc2bc86bb358a3b22f1

push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=7e58207e220a837a79af6101c6fdfb14bb84a828

failure log: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=7e58207e220a837a79af6101c6fdfb14bb84a828

task 2018-08-15T21:00:57.371Z] TEST-PASS | non262/RegExp/regress-334158.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.0 s]
[task 2018-08-15T21:00:57.371Z] {"action": "test_start", "jitflags": [], "pid": 50365, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "test": "non262/RegExp/regress-334158.js", "thread": "main", "time": 1534366857.330933}
[task 2018-08-15T21:00:57.371Z] {"action": "test_end", "extra": {"jitflags": [], "pid": 50365, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so"}, "jitflags": [], "pid": 50365, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "status": "PASS", "test": "non262/RegExp/regress-334158.js", "thread": "main", "time": 1534366857.371726}
[task 2018-08-15T21:00:57.383Z] ## non262/RegExp/oom-in-construction.js: rc = 0, run time = 0.050823
[task 2018-08-15T21:00:57.383Z] BUGNUMBER: 1471371
[task 2018-08-15T21:00:57.383Z] STATUS: Handle OOM in RegExp
[task 2018-08-15T21:00:57.383Z] TEST-UNEXPECTED-FAIL | non262/RegExp/oom-in-construction.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.1 s]
[task 2018-08-15T21:00:57.383Z] {"action": "test_start", "jitflags": [], "pid": 50398, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "test": "non262/RegExp/oom-in-construction.js", "thread": "main", "time": 1534366857.332639}
[task 2018-08-15T21:00:57.383Z] {"action": "test_end", "extra": {"jitflags": [], "pid": 50398, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so"}, "jitflags": [], "pid": 50398, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "status": "FAIL", "test": "non262/RegExp/oom-in-construction.js", "thread": "main", "time": 1534366857.383462}
[task 2018-08-15T21:00:57.384Z] TEST-PASS | non262/RegExp/RegExp_leftContext_as_array.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.0 s]
[task 2018-08-15T21:00:57.384Z] {"action": "test_start", "jitflags": [], "pid": 50403, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "test": "non262/RegExp/RegExp_leftContext_as_array.js", "thread": "main", "time": 1534366857.3369}
[task 2018-08-15T21:00:57.384Z] {"action": "test_end", "extra": {"jitflags": [], "pid": 50403, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so"}, "jitflags": [], "pid": 50403, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "status": "PASS", "test": "non262/RegExp/RegExp_leftContext_as_array.js", "thread": "main", "time": 1534366857.384053}
[task 2018-08-15T21:00:57.384Z] TEST-PASS | non262/RegExp/regress-165353.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.0 s]
Flags: needinfo?(khyperia)
(Assignee)

Comment 6

4 months ago
Created attachment 9001428 [details] [diff] [review]
OOM handling in RegExp construction
(Assignee)

Updated

4 months ago
Attachment #8999973 - Attachment is obsolete: true
Flags: needinfo?(khyperia)
(Assignee)

Updated

4 months ago
Keywords: checkin-needed

Comment 8

4 months ago
Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8464c338715d
OOM handling in RegExp construction. r=jorendorff
Keywords: checkin-needed

Comment 9

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8464c338715d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.