"Assertion failure: OBJ_GET_CLASS(cx, obj) == &js_ArrayClass" with ternary and array comprehension

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, {crash, testcase, verified1.8.1})

Trunk
PowerPC
Mac OS X
crash, testcase, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.0.8 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:investigate] js1.7)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

12 years ago
Steps to reproduce:
  In a debug build,
  javascript:void ({y: true ? [1 for (x in [2])] : 3 })

Result:
  Assertion failure: OBJ_GET_CLASS(cx, obj) == &js_ArrayClass, at js/src/jsinterp.c:6090
I can't reproduce this anymore. I'm optimistically marking it as a dupe of bug 349493.

*** This bug has been marked as a duplicate of 349493 ***
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
(Reporter)

Comment 2

12 years ago
I can still reproduce sometimes with the original steps.  Also, I can reproduce reliably with...

Steps to reproduce:
  In a debug build,
  javascript:let (a) true ? [2 for each (z in function(id) { return id })] : 3;

Result: 
  jsinterp.c:6081 crashes or asserts.
  JS_ASSERT(OBJ_GET_CLASS(cx, obj) == &js_ArrayClass);

obj has been observed to be 0x00000000 or 0x80000000 (causing a segfault).  When it asserts, I'm guessing it "obj" is just a bogus address that can be dereferenced without segfaulting.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Whiteboard: [sg:investigate]
(Assignee)

Comment 3

12 years ago
Created attachment 236194 [details] [diff] [review]
fix

So the constant folder has to update the unique down-pointer from parent to child when it moves a JSParseNode due to a constant condition selecting a then or else expression or clause.  But for TOK_ARRAYCOMP, there's an up-pointer several nodes down in the TOK_ARRAYPUSH node, pn_array, that needs fixing.

Otherwise we never point from two nodes to one -- the AST is a tree except for this cyclic up link, and there's no need for multiple connections in general.  So I'm not going to mark a FIXME here, since I do not expect more up-pointers to be added to JSParseNode, so there's no point in generalizing.

/be
Assignee: general → brendan
Status: REOPENED → ASSIGNED
Attachment #236194 - Flags: review?(mrbkap)

Updated

12 years ago
Attachment #236194 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 4

12 years ago
Created attachment 236340 [details] [diff] [review]
altern-patch I like better

This preserves the single-pointer-to-JSParseNode-from-any-other-JSParseNode (where the other node is always the parent, and pn_next does not count for the constant ? or if condition case) property.  If you like it better too, please retract r+ on other patch and obsolete it, and r+ this one.  It fixes the bug, using the nearest C equivalent to a statically scoped outer function variable (cg->arrayCompSlot).

/be
Attachment #236340 - Flags: review?(mrbkap)
Comment on attachment 236340 [details] [diff] [review]
altern-patch I like better

Yeah, this is better.
Attachment #236340 - Flags: review?(mrbkap) → review+

Updated

12 years ago
Attachment #236194 - Attachment is obsolete: true
Attachment #236194 - Flags: review+
(Assignee)

Comment 6

12 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #236340 - Flags: approval1.8.1?
(In reply to comment #2)

> 
> obj has been observed to be 0x00000000 or 0x80000000 (causing a segfault). 

i believe in some cases 0x80000000 may point to used heap - if one consumes enough memory.
Comment on attachment 236340 [details] [diff] [review]
altern-patch I like better

a=beltzner on behalf of 181drivers
Attachment #236340 - Flags: approval1.8.1? → approval1.8.1+
on linux 2.6 creating 2 js with byte size 256MB each makes 0x80000000 mapped on the heap.

Comment 10

12 years ago
Checking in regress-349653.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-349653.js,v  <--  regress-349653.js
initial revision: 1.1
Flags: in-testsuite+
(Assignee)

Comment 11

12 years ago
Fixed on the 1.8 branch too.

/be
Keywords: fixed1.8.1

Comment 12

12 years ago
verified fixed 1.9 20060901 windows/mac*/linux
Status: RESOLVED → VERIFIED

Comment 13

12 years ago
verified fixed 1.8 2006090118 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1

Comment 14

12 years ago
the test was originally misplaced.
Removing js1_5/Regress/regress-349653.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-349653.js,v  <--  regress-349653.js
new revision: delete; previous revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_7/block/regress-349653.js,v
done
Checking in js1_7/block/regress-349653.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-349653.js,v  <--  regress-349653.js
initial revision: 1.1
done
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8? → blocking1.8.0.8-
Whiteboard: [sg:investigate] → [sg:investigate] js1.7
You need to log in before you can comment on or make changes to this bug.