Closed Bug 1505811 Opened 4 years ago Closed 4 years ago

Assertion failure: !JS_IsExceptionPending(cx), at /builds/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:3779


(Core :: DOM: Core & HTML, defect, P1)




Tracking Status
firefox-esr60 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed


(Reporter: jkratzer, Assigned: bzbarsky)


(Blocks 1 open bug, )


(Keywords: assertion, testcase)


(2 files)

Attached file testcase.html
Testcase found while fuzzing mozilla-central rev b3da3f53f804.

Assertion failure: !JS_IsExceptionPending(cx), at /builds/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:3779

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007ffdc8d8bbb0
rsi = 0x00007f4afa5d38b0   rdi = 0x00007f4afa5d2680
rbp = 0x00007ffdc8d8bc70   rsp = 0x00007ffdc8d8bb40
r8 = 0x00007f4afa5d38b0    r9 = 0x00007f4afb744740
r10 = 0x0000000000000002   r11 = 0x0000000000000000
r12 = 0x00007ffdc8d8bb90   r13 = 0x00007f4aef50e401
r14 = 0x00007ffdc8d8bb70   r15 = 0x00007f4adfc13550
rip = 0x00007f4ae9ecf85f
OS|Linux|0.0.0 Linux 4.15.0-38-generic #41-Ubuntu SMP Wed Oct 10 10:59:38 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
0|1||bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy>(JSContext*, unsigned int, JS::Value*)||3318|0x11
Flags: in-testsuite?
Hi Ehsan, sorry to bother you. As ElementBinding.cpp is automatically generated, I'm unsure who I could ping about this for triage purposes. Any suggestions?
Flags: needinfo?(ehsan)
Priority: -- → P3
This would be a bug in the bindings code as a rough first guess.  Redirecting to bz (peterv would be a good second guess.)
Flags: needinfo?(ehsan) → needinfo?(bzbarsky)
Generally means a bug in some code bindings called, which left an exception on the JSContext but didn't bother to tell the caller.  I'll look.
rr makes debugging these things so much nicer...

The broken code is in nsContentUtils::IsPatternMatching.  It does this:

  if (!JS_ExecuteRegExpNoStatics(cx, re,
                                 aValue.Length(), &idx, true, &rval)) {
    return true;

The testcase does a bunch of stack exhaustion (via a connectedCallback, which triggers a DOM modification which (is this part correct???) triggers another connectedCallback invocation before the previous one returned, etc).  It also triggers validity checks via the <input pattern=""value=ð> bit.  The regexp execution fails because of the nearly-exhausted stack (with a "regexp too big" error), but the caller never clears that exception from the JSContext.

And this is why AutoJSContext is evil and should die.  If IsPatternMatching used AutoJSAPI, it would report the exception automatically before returning!
Oh, and we used to have a JS_ClearPendingException before that "return true", added in bug 709954, precisely because of this problem.  The JS_ClearPendingException was removed in bug 1112040, when the JSAPI took over  error reporting, because we _did_ want to report those exceptions to the console.

And then bug 1379585 removed the AutoJSAPI and added AutoJSContext, which people should _never_ be doing.  Ever.

The test for bug 709954 didn't start failing in the process, because it tests the "failed to compile" codepath, which properly drops the exceptions due to the code that was added in bug 1235159 (which reports a nicer error to the console).  But this bug's testcase exercises the "failed to execute" codepath, which just leaves the  exception sitting there.
Blocks: 1379585
Flags: needinfo?(bzbarsky)
Priority: P3 → P1
Pushed by
Don't leave exceptions dangling on the JSContext when regexp execution fails during HTML input pattern matching.  r=baku
Looks Boris has been working on this, so setting assignee accordingly :)
Assignee: nobody → bzbarsky
This is long-since fixed.  Why wasn't it resolved?  :(
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: in-testsuite? → in-testsuite+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.