"Assertion failure: next == JSOP_IFNE || next == JSOP_IFNEX," with trap

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
x86
Linux
assertion, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: js-triage-needed)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
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

6 years ago
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

6 years ago
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

6 years ago
(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

6 years ago
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.
(Reporter)

Comment 6

6 years ago
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
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.