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

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: 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
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|0
0|0|libxul.so|mozilla::dom::Element_Binding::set_outerHTML|s3:gecko-generated-sources:b5a8f0837809e950a043caaec1200492046890c4fcaa965cc5e5d5149b855d0dbe2972a461d957517675dea912e83d04f9a29c07cd777afb9f38efa27702d948/dom/bindings/ElementBinding.cpp:|3779|0x18
0|1|libxul.so|bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy>(JSContext*, unsigned int, JS::Value*)|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:b3da3f53f8042d6e2e8f90cd0086e354d96ba2fc|3318|0x11
0|2|||||0xe1ed64a9949
0|3|libxul.so|EnterJit|hg:hg.mozilla.org/mozilla-central:js/src/jit/Jit.cpp:b3da3f53f8042d6e2e8f90cd0086e354d96ba2fc|105|0x29
0|4|libxul.so||||0x3b821d0
0|5|libxul.so|_fini|||0xe29ab8

Comment 1

[Marcos Caceres [:marcosc]]

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?

Comment 2

[(no longer active)]

This would be a bug in the bindings code as a rough first guess.  Redirecting to bz (peterv would be a good second guess.)

Comment 3

[Boris Zbarsky [: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.

Comment 4

[Boris Zbarsky [:bzbarsky]]

rr makes debugging these things so much nicer...

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

  if (!JS_ExecuteRegExpNoStatics(cx, re,
                                 static_cast<char16_t*>(aValue.BeginWriting()),
                                 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!

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1505811. Don't leave exceptions dangling on the JSContext when regexp execution fails during HTML input pattern matching. r=baku

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2170be698d4b
Don't leave exceptions dangling on the JSContext when regexp execution fails during HTML input pattern matching.  r=baku

Comment 8

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Looks Boris has been working on this, so setting assignee accordingly :)

Comment 9

[Boris Zbarsky [:bzbarsky]]

This is long-since fixed.  Why wasn't it resolved?  :(