980384 - Assertion failure: slot < numValueSlots(), at jit/BaselineFrame.h:150 or Bus Error

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 8122ffa9e1aa (run with --fuzzing-safe):


function testcase() {
  try {
    testcase('(function (){var arguments;});');
  }
  catch (e) {
    return (testcase(0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9));
  }
}
testcase(testcase);

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

Crashes with a bus error:

Program received signal SIGBUS, Bus error.
0x00007ffff7ff1c4c in ?? ()
(gdb) x /i $pc
=> 0x7ffff7ff1c4c:      testl  $0x200,-0x8(%rbp)
(gdb) info reg rbp
rbp            0xfff9000000000000       0xfff9000000000000


Marked s-s based on that.

Comment 3

[Nicolas B. Pierron [:nbp]]

I would expect that we bailout from the try block when we exceed the recursion limit, but I do not see where the catch block is important here.

Jandem might be the right person to ask about this issue.

Comment 4

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140225091817" and the hash "ecfb4d63943f".
The "bad" changeset has the timestamp "20140225092218" and the hash "cc698e2ff036".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ecfb4d63943f&tochange=cc698e2ff036

Comment 5

[Christian Holler (:decoder)]

Needinfo from wingo based on comment 4, likely regressed by bug 962599.

Comment 6

[Andy Wingo [:wingo]]

Attachment: Fix frame marker for half-constructed stack frames

Comment 7

[Andy Wingo [:wingo]]

Comment on attachment 8388401 [details] [diff] [review]
Fix frame marker for half-constructed stack frames

Unfortunately I was unable to produce a test case.  Your test case recurses until it reaches the stack limit, then hammers at the stack limit with a recursive function with many locals until GC happens during an early stack check.  There's no obvious way to make the test terminate, at least that I can see, and it's going to be n-squared on Mac due to its larger stack size and the cost of constructing the exception.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8388401 [details] [diff] [review]
Fix frame marker for half-constructed stack frames

Review of attachment 8388401 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.

Comment 9

[Andy Wingo [:wingo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7450b87d6ff4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This shouldn't have landed without security approval since it doesn't have a rating and isn't clear what branches it affects.
https://hg.mozilla.org/mozilla-central/rev/7450b87d6ff4

Comment 11

[Al Billings [:abillings - ex-MoCo]]

https://wiki.mozilla.org/Security/Bug_Approval_Process

Comment 12

[Jesse Ruderman]

> There's no obvious way to make the test terminate, at least that I can see, 
> and it's going to be n-squared on Mac due to its larger stack size and the 
> cost of constructing the exception.

Perhaps you could fix bug 813646 and/or bug 735082?

I've stopped fuzzing these too-much-recursion corners because even fuzz testcases are slow and unreliable. Meanwhile, Decoder is finding plenty of bugs with this pattern (e.g. this bug, bug 978811, bug 970252, bug 957716).

Comment 13

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 14

[Andy Wingo [:wingo]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #10)
> This shouldn't have landed without security approval since it doesn't have a
> rating and isn't clear what branches it affects.
> https://hg.mozilla.org/mozilla-central/rev/7450b87d6ff4

As I understand the guidelines, this was simply a recent regression in m-c that never shipped, and so landing it directly was OK.