Closed Bug 1685290 Opened 5 years ago Closed 5 years ago

Assertion failure: script->hasIonScript(), at jit/Ion.cpp:2685 with invalidate

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
86 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- unaffected
firefox85 --- wontfix
firefox86 --- verified

People

(Reporter: decoder, Assigned: iain)

References

(Regression)

Details

(4 keywords, Whiteboard: [bugmon:update,bisected,confirmed])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 20210106-1e323e0a130c (build with --enable-debug, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --ion-full-warmup-threshold=0 --more-compartments --fast-warmup):

var target  = newGlobal({newCompartment: true});
function testRegExpDollar() {
  var s19 = "Foobar123bar2345";
  var re1 = /bar\\d\\d/;
  close = new Proxy({}, { get: target.invalidate });
  s19.replace(re1, eval(close));
}
function ut(s) {
  return s.replace(/x/g, 'y').replace(/x/g, 'y').replace(/x/g, 'y');
}
for (var idx = 0; idx < 5; ++idx) {
  try {
    testRegExpDollar();
  } catch (lfVare) {
    ut("x");
  }
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00005555578b95b6 in js::jit::Invalidate(JSContext*, JSScript*, bool, bool) ()
#0  0x00005555578b95b6 in js::jit::Invalidate(JSContext*, JSScript*, bool, bool) ()
#1  0x00005555570d412e in testingFunc_invalidate(JSContext*, unsigned int, JS::Value*) ()
#2  0x0000555556b9fbe2 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#3  0x0000555556b9f49a in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#4  0x0000555556ba0824 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) ()
#5  0x0000555556ba0a60 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) ()
#6  0x0000555556ce1bb6 in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const ()
#7  0x0000555556ca9664 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const ()
#8  0x0000555556cd0b26 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) ()
#9  0x0000555556b9f607 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#10 0x0000555556ba0824 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) ()
#11 0x0000555556ba0a60 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) ()
#12 0x0000555556cd985e in js::ScriptedProxyHandler::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) const ()
#13 0x0000555556ccef90 in js::Proxy::getInternal(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) ()
#14 0x0000555556ccebae in js::Proxy::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) ()
#15 0x0000555556eaf27b in MaybeCallMethod(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) ()
#16 0x0000555556eaeb2c in JS::OrdinaryToPrimitive(JSContext*, JS::Handle<JSObject*>, JSType, JS::MutableHandle<JS::Value>) ()
#17 0x0000555556eaf8dd in js::ToPrimitiveSlow(JSContext*, JSType, JS::MutableHandle<JS::Value>) ()
#18 0x0000555556fe2848 in JSString* js::ToStringSlow<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType) ()
#19 0x0000129ac2f77ba5 in ?? ()
#20 0x0000000000000000 in ?? ()
rax	0x555555808b10	93824995068688
rbx	0x7fffffff98d0	140737488328912
rcx	0x555557fe9a08	93825036884488
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffff98c0	140737488328896
rsp	0x7fffffff9840	140737488328768
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f998c0	140737353717952
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7ffff60ed400	140737321554944
r13	0x7fffffff9f10	140737488330512
r14	0x7ffff6024000	140737320730624
r15	0x7fffffff9f70	140737488330608
rip	0x5555578b95b6 <js::jit::Invalidate(JSContext*, JSScript*, bool, bool)+662>
=> 0x5555578b95b6 <_ZN2js3jit10InvalidateEP9JSContextP8JSScriptbb+662>:	movl   $0xa7d,0x0
   0x5555578b95c1 <_ZN2js3jit10InvalidateEP9JSContextP8JSScriptbb+673>:	callq  0x555556a9711a <abort>

This could be a problem with the invalidate shell-builtin but I don't know for sure and since this is a JIT assertion, marking s-s.

Also, it would be helpful to know why the outer for-loop is required and if there is a combination of options that allows to get rid of it entirely (or are at least 2 iterations necessary in any case?).

Attached file Testcase —

This is just a problem with the invalidate builtin. If we call ToStringSlow from a warp-compiled script (in this case, self-hosted RegExpReplace) on a proxy whose get method is invalidate, then ToPrimitiveSlow will hit the proxy trap at least twice. The first one will invalidate the caller, and the second one will assert because it expects the caller to still be valid. The fix is to only call Invalidate in testingFunc_invalidate if the IonScript still exists.

Here's a cut-down testcase (with --no-threads and --ion-eager):

with ({}) {}

function test(s) {
    "".replace(/x/, s);
}

// Warm up RegExpReplace so that it doesn't bail before calling ToStringSlow
for (var i = 0; i < 20; i++) {
    test("");
}

try {
    test(new Proxy({}, { get: invalidate }));
} catch {}
Assignee: nobody → iireland
Group: javascript-core-security

If we call ToStringSlow from a warp-compiled script (in this case, self-hosted RegExpReplace) on a proxy whose get method is the invalidate testing function, then ToPrimitiveSlow will hit the proxy trap at least twice. The first one will invalidate the caller, and the second one will assert because it expects the caller to still be valid. The fix is to only call Invalidate in testingFunc_invalidate if the IonScript still exists.

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210108215736-e7ad4ad7c65b.
The bug appears to have been introduced in the following build range:

Start: 08d764beb3dda67899b7c1b11748414c728b3e67 (20201211220903)
End: 5a21d06dbf403745e9c48fbdcedc3156d7144a9c (20201211221127)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=08d764beb3dda67899b7c1b11748414c728b3e67&tochange=5a21d06dbf403745e9c48fbdcedc3156d7144a9c

Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisected,confirmed]
Pushed by iireland@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae2c42cffc1c Check for IonScript before invalidating r=jandem
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210112043340-b97f7d02c912.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
Flags: in-testsuite+
Regressed by: 1681806
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: