1492574 - Assertion failure: !cx->isExceptionPending() with OOM and GetDynamicName

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Iain Ireland [:iain]]

GetDynamicName calls AtomizeString, which may return nullptr in case of OOM, but does not clear pending exceptions. This leaks the exception flag, which triggers assertion failures in subsequent VM calls with AutoAssertNoPendingException guards.

The following testcase crashes:

function foo() {}
function foooooooooooooooooooooooooooooooo() {}
function fn(s) {
    var o = {a:1};
    eval(("f" + s) + "()");
     if (!("a" in o)) {
        print("unreachable");
     }
}
for (var i = 0; i < 1100; i++) {
    with ({}) {}
    fn("oo");
}
oomTest(new Function(`
fn(newRope("oooooooooooooooo", "oooooooooooooooo"));
`));

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 1492574 - Avoid leaking OOM exception flag from GetDynamicName r?tcampbell

Comment 2

[Ted Campbell [:tcampbell]]

Comment on attachment 9010686 [details]
Bug 1492574 - Avoid leaking OOM exception flag from GetDynamicName r?tcampbell

Ted Campbell [:tcampbell] has approved the revision.

Comment 3

[Ted Campbell [:tcampbell]]

Comment on attachment 9010686 [details]
Bug 1492574 - Avoid leaking OOM exception flag from GetDynamicName r?tcampbell

Ted Campbell [:tcampbell] has requested changes to the revision.

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1492574: Fix OOM handling in NewRope testing function r?tcampbell

Depends on D6404

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1492574: Rewrite GetDynamicName to return false if lookup can't be completed r?tcampbell

Depends on D7122

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1492574: Remove unnecessary MutableHandle r?tcampbell

Depends on D7123

Comment 7

[Ted Campbell [:tcampbell]]

Comment on attachment 9012695 [details]
Bug 1492574: Remove unnecessary MutableHandle r?tcampbell

Ted Campbell [:tcampbell] (PTO until Oct 9) has approved the revision.

Comment 8

[Ted Campbell [:tcampbell]]

Comment on attachment 9012693 [details]
Bug 1492574: Fix OOM handling in NewRope testing function r?tcampbell

Ted Campbell [:tcampbell] (PTO until Oct 9) has approved the revision.

Comment 9

[Ted Campbell [:tcampbell]]

Comment on attachment 9012694 [details]
Bug 1492574: Rewrite GetDynamicName to return false if lookup can't be completed r?tcampbell

Ted Campbell [:tcampbell] (PTO until Oct 9) has approved the revision.

Comment 10

[Ted Campbell [:tcampbell]]

Comment on attachment 9010686 [details]
Bug 1492574 - Avoid leaking OOM exception flag from GetDynamicName r?tcampbell

Ted Campbell [:tcampbell] (PTO until Oct 9) has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23bab6bcd81c
Avoid leaking OOM exception flag from GetDynamicName r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/414785cda258
Fix OOM handling in NewRope testing function r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/dfdc496fab45
Rewrite GetDynamicName to return false if lookup can't be completed r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/5109c878f7e2
Remove unnecessary MutableHandle r=tcampbell

Comment 12

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/23bab6bcd81c
https://hg.mozilla.org/mozilla-central/rev/414785cda258
https://hg.mozilla.org/mozilla-central/rev/dfdc496fab45
https://hg.mozilla.org/mozilla-central/rev/5109c878f7e2