1282047 - Crash [@ js::Wrapper::isArray] or Crash [@ js::ScriptedProxyHandler::isArray] due to over-recursion

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 939ecc4e9d05 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe):

function assertThrowsInstanceOf() f;
global = this;
otherGlobal = newGlobal();
alternateGlobals = function(i) {
    return () => i++ % 2 ? global : otherGlobal;
}(0);
function performTests(pickGlobal) {
    revocable = Proxy.revocable([], {});
    proxy = revocable;
    for (i = 0; i < 70000; i++)
        proxy = new(pickGlobal()).Proxy(proxy, {});
    assertThrowsInstanceOf(Array.isArray(proxy));
}
performTests(alternateGlobals);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:250
#0  js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:250
#1  0x0000000000a02a18 in js::ScriptedProxyHandler::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/ScriptedProxyHandler.cpp:1254
#2  0x0000000000a03bd3 in js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:251
#3  0x0000000000a02a18 in js::ScriptedProxyHandler::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/ScriptedProxyHandler.cpp:1254
#4  0x0000000000a03bd3 in js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:251
#5  0x0000000000a02a18 in js::ScriptedProxyHandler::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/ScriptedProxyHandler.cpp:1254
#6  0x0000000000a03bd3 in js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:251
#7  0x0000000000a02a18 in js::ScriptedProxyHandler::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/ScriptedProxyHandler.cpp:1254
#8  0x0000000000a03bd3 in js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:251
[...]
#126 0x0000000000a03bd3 in js::Wrapper::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/Wrapper.cpp:251
#127 0x0000000000a02a18 in js::ScriptedProxyHandler::isArray (this=<optimized out>, cx=0x7ffff691ac00, proxy=..., answer=0x7fffffffc134) at js/src/proxy/ScriptedProxyHandler.cpp:1254
rax	0x0	0
rbx	0x7fffff7ff040	140737479962688
rcx	0x7fffffffc134	140737488339252
rdx	0x7fffff7ff050	140737479962704
rsi	0x7ffff691ac00	140737330129920
rdi	0x7ffff2da22a0	140737267770016
rbp	0x7fffff7ff030	140737479962672
rsp	0x7fffff7feff0	140737479962608
r8	0xffffff00	4294967040
r9	0x7ffff6a00f38	140737331072824
r10	0x7ffff6953000	140737330360320
r11	0x206	518
r12	0x7ffff691ac00	140737330129920
r13	0x7fffffffc134	140737488339252
r14	0x0	0
r15	0x7fffffffc650	140737488340560
rip	0xa03b85 <js::Wrapper::isArray(JSContext*, JS::Handle<JSObject*>, JS::IsArrayAnswer*) const+37>
=> 0xa03b85 <js::Wrapper::isArray(JSContext*, JS::Handle<JSObject*>, JS::IsArrayAnswer*) const+37>:	callq  0x7e7b10 <JSObject::as<js::ProxyObject>()>
   0xa03b8a <js::Wrapper::isArray(JSContext*, JS::Handle<JSObject*>, JS::IsArrayAnswer*) const+42>:	mov    %rax,%rdi

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This seems to go back beyond m-c rev dc4b163f7db7 (early Nov 2014).

Setting needinfo? from :jorendorff and :Waldo as a start, please feel free to move the NI? on where necessary.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Patch, has r=efaust over IRL

For such a trivial change I just got efaust to shoulder-review this.  Would land now, but tree's in approval-needed state, so queueing up as ridealong with a checkin-needed sweep.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9747585b294a
Do a recursion check in Proxy::isArray to deal with proxies whose isArray trap recursively consumes too much stack. r=efaust over IRL

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d1b225b7a5a
Trim a loop limit from 5e5 to 1e5 so as to hopefully not time out on cgc builds.  r=red, a=KWierso

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9747585b294a
https://hg.mozilla.org/mozilla-central/rev/8d1b225b7a5a

Comment 6

[Andrei Vaida [:avaida]]

I believe we can safely mark this verified fixed on Fx50, based on the last ~3 months of crash data.

  SIGNATURE   | js::Wrapper::isArray
  ----------------------------------------
  CRASH STATS | http://tinyurl.com/gwwfp5u
  ----------------------------------------
  OVERVIEW    | 0 crashes on nightly 52
	      | 0 crashes on nightly 51
	      | 0 crashes on aurora 51
	      | 0 crashes on nightly 50
	      | 0 crashes on aurora 50
	      | 0 crashes on beta 50


  SIGNATURE   | js::ScriptedProxyHandler::isArray
  -----------------------------------------------
  CRASH STATS | http://tinyurl.com/zmchkw2
  -----------------------------------------------
  OVERVIEW    | 0 crashes on nightly 52
	      | 0 crashes on nightly 51
	      | 0 crashes on aurora 51
	      | 0 crashes on nightly 50
	      | 0 crashes on aurora 50
	      | 0 crashes on beta 50