520591 - "Assertion failure: invalid RECURSIVE_MISMATCH exit" with recursion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Attachment: fragile testcase

With hg rev 9adc8fa19990+, this fragile testcase triggers an assertion failure.  It's so fragile I can't even reproduce under Valgrind!

~/tracemonkey/js/src/debug/js -j f1.js 

Assertion failure: invalid RECURSIVE_MISMATCH exit, at ../jsrecursion.cpp:72

Comment 1

[Jesse Ruderman]

WFM, tracemonkey rev 814e6dd2c1a3+.

Comment 2

[Jesse Ruderman]

Attachment: fragile testcase for tm rev 2f1bf725cfff+

Comment 3

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

Attachment: Separate somewhat reduced fragile testcase for tm rev 2f1bf725cfff+

$ cat 1testcase.js 
function() {}
try { (function() { (function() {
            t1
        })
    })
} catch(e) {}
function() {}
function() {}
function() {}
(function() { (x
        for (x in []) if (3))
    })
function() {}
function() {}
function() {}
function() {}
function() {}
function() {}
(function() { ((function() {
            (function() { [ou] } [MI], 0)
        })(el))
    })
try { (function() {
        for (a = 1; a < 6; ++a) {
            if (a == 0) {} else {
                print((function sum_slicing(aaaaaa) {
                    return aaaaaa.length == 0 ? 0 : aaaaaa[0] + sum_slicing(aaaaaa.slice(1))
                })(['', (0), (0), (0), (0), (0), 'AA', (-0), null, (-1)]))
            }
        }
    })()
} catch(e) {}
try { (function() {})() } catch(e) {}
try { (function() { (function foo(bbbbbb) {
            if (bbbbbb < 1) {
                return 1
            }
            return foo(bbbbbb - 1) + foo(bbbbbb - 2)
        })(6)
    })()
} catch(e) {}
$ ./js-dbg-tm-darwin -j 1testcase.js 
00000AA-1
00000AA-1
00000AA-1
00000AA-1
00000AA-1
Assertion failure: invalid RECURSIVE_MISMATCH exit, at ../jsrecursion.cpp:72
Trace/BPT trap

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

I can reproduce this - interestingly, on x86 but not x64.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

The purpose of this assert is to make sure that the FrameInfo that restored the stack has the same types as if we capture the stack again.

Problem is the Oracle can result in int32/double mismatches that are really harmless. The point of this assert is to catch the really bad things, like the stack height differing or a completely different JS type.

Comment 6

[Andreas Gal :gal]

Comment on attachment 404971 [details] [diff] [review]
fix

>diff --git a/js/src/jsrecursion.cpp b/js/src/jsrecursion.cpp
>--- a/js/src/jsrecursion.cpp
>+++ b/js/src/jsrecursion.cpp
>@@ -62,18 +62,20 @@ AssertDownFrameIsConsistent(JSContext* c
>     unsigned downPostSlots = fi->callerHeight;
>     JSTraceType* typeMap = fi->get_typemap();
> 
>     js_CaptureStackTypes(cx, 1, typeMap);
>     const JSTraceType* m1 = anchor->recursive_down->get_typemap();
>     for (unsigned i = 0; i < downPostSlots; i++) {
>         if (m1[i] == typeMap[i])
>             continue;
>-        if (typeMap[i] == TT_INT32 && m1[i] == TT_DOUBLE)
>+        if (typeMap[i] == TT_INT32 && m1[i] == TT_DOUBLE ||
>+            typeMap[i] == TT_DOUBLE && m1[i] == TT_INT32) {
>             continue;

Copy into temporaries please. It read over and over from the array.

>+        }
>         JS_NOT_REACHED("invalid RECURSIVE_MISMATCH exit");
>     }
>     JS_ASSERT(memcmp(anchor->recursive_down, fi, sizeof(FrameInfo)) == 0);
> }
> #endif
> 
> JS_REQUIRES_STACK VMSideExit*
> TraceRecorder::downSnapshot(FrameInfo* downFrame)
>diff --git a/js/src/trace-test/tests/basic/testBug520591.js b/js/src/trace-test/tests/basic/testBug520591.js
>new file mode 100644
>--- /dev/null
>+++ b/js/src/trace-test/tests/basic/testBug520591.js
>@@ -0,0 +1,45 @@
>+function() {}
>+try { (function() { (function() {
>+            t1
>+        })
>+    })
>+} catch(e) {}
>+function() {}
>+function() {}
>+function() {}
>+(function() { (x
>+        for (x in []) if (3))
>+    })
>+function() {}
>+function() {}
>+function() {}
>+function() {}
>+function() {}
>+function() {}
>+(function() { ((function() {
>+            (function() { [ou] } [MI], 0)
>+        })(el))
>+    })
>+try { (function() {
>+        for (a = 1; a < 6; ++a) {
>+            if (a == 0) {} else {
>+                print((function sum_slicing(aaaaaa) {
>+                    return aaaaaa.length == 0 ? 0 : aaaaaa[0] +
>+sum_slicing(aaaaaa.slice(1))
>+                })(['', (0), (0), (0), (0), (0), 'AA', (-0), null, (-1)]))
>+            }
>+        }
>+    })()
>+} catch(e) {}
>+try { (function() {})() } catch(e) {}
>+try { (function() { (function foo(bbbbbb) {
>+            if (bbbbbb < 1) {
>+                return 1
>+            }
>+            return foo(bbbbbb - 1) + foo(bbbbbb - 2)
>+        })(6)
>+    })()
>+} catch(e) {}
>+
>+/* Don't assert */
>+

Comment 7

[Andreas Gal :gal]

Thats one awesome testcase. Thanks jesse for teaching the fuzzer about recursion so quickly.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/140346641114

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/140346641114