Last Comment Bug 678546 - "Assertion failure: next == JSOP_IFNE || next == JSOP_IFNEX," with trap
: "Assertion failure: next == JSOP_IFNE || next == JSOP_IFNEX," with trap
Status: RESOLVED FIXED
js-triage-needed
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: ---
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2011-08-12 10:40 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2011-12-09 14:53 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
dis() information, and stack (6.67 KB, text/plain)
2011-08-12 10:40 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details

Description Gary Kwong [:gkw] [:nth10sd] 2011-08-12 10:40:43 PDT
Created attachment 552689 [details]
dis() information, and stack

function f() {
    for (let x in [, 0, , 0, 0, , x = x]) {
        t((p = {}, (z(x('')))()))
    }
}
dis(f)
trap(f, 106, '')
f()

asserts js debug shell on MI changeset 223d4f4bd252 with patch v1 from bug 672892 with -m, -a and -d at Assertion failure: next == JSOP_IFNE || next == JSOP_IFNEX,

This assert happens pretty frequently during fuzzing rounds.
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2011-10-18 15:34:25 PDT
This assert was added by jandem in bug 655505, but I'm not sure if it's the exact cause of the assert above, which still happens in m-c changeset 7ed661aa832d.

It'll be nice to have this on the radar.
Comment 2 Jim Blandy :jimb 2011-11-10 13:21:13 PST
Where did the fuzzer get the '106' from? I don't think we need to test setting traps at every bytecode. Just zero and the ones returned by JS_LineNumberToPC and functions like that should be enough: statement boundaries, "sequence points". I'm pretty sure the bytecode this test sets a trap at is not one of those things.
Comment 3 Jesse Ruderman 2011-11-10 14:14:47 PST
The fuzzer gets trap offsets from the left column of disassemble().

Is there a way to get all the sequence points? I'm worried that using just JS_LineNumberToPC would miss a lot of them.

(Related: bug 396512, bug 429239)
Comment 4 Jim Blandy :jimb 2011-11-14 13:01:15 PST
(In reply to Jesse Ruderman from comment #3)
> The fuzzer gets trap offsets from the left column of disassemble().

This is reasonable, but more fine-grained than any debugger actually needs. There's no way for the debugger to get those line-interior offsets.

> Is there a way to get all the sequence points? I'm worried that using just
> JS_LineNumberToPC would miss a lot of them.

Why do we need to test all those sequence points? I mean, I grant that it's reasonable to want to set a breakpoint before the second assignment in:

    i=1; j=2

but there's no way to find the bytecode offset of that second assignment. And for the fuzzer's sake, it could simply put that second assignment on a different line, and then JS_LineNumberToPC would provide it. (Or, equivalently, parsing the 'source notes' part of the 'dis' output.)

We are going to add column information (bug 568142) but that will not be giving every token its own bytecode offset; it'll still make distinctions on a statement granularity or something meaningful like that. But the point is, we only need to test traps set on *discoverable* bytecode offsets.

Now, perhaps it's easy for a JM person to fix this assertion. I'm just saying that the debugger doesn't really need the functionality, so if it's not trivial, then the fuzzer should be changed.
Comment 5 Jim Blandy :jimb 2011-11-14 13:07:38 PST
As far as I know, the shell is not meant to be exposed to untrusted code. It's got all sorts of impressively powerful primitives --- "resolver", for example, can cause interesting kinds of trouble. If it's meant to be suitable for untrusted code, then we really need to do an audit.
Comment 6 Gary Kwong [:gkw] [:nth10sd] 2011-12-09 14:53:49 PST
Probably fixed by bug 707454.

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   82208:dfd8e10f7155
user:        Brian Hackett
date:        Wed Dec 07 13:15:48 2011 -0800
summary:     Remove JSOP_TRAP, bug 707454. r=jorendorff

Note You need to log in before you can comment on or make changes to this bug.