"Assertion failure: !vp->isPrimitive()"

RESOLVED FIXED

Status

()

defect
--
critical
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jruderman, Assigned: luke)

Tracking

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

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

9 years ago
Testcase:
  new (Proxy.createFunction({}, function(){}, function(){}));

Result:
  Assertion failure: !vp->isPrimitive(), at jscntxtinlines.h:632

The first bad revision is:
  changeset:   52720:66c8ad02543b
  user:        Luke Wagner <lw@mozilla.com>
  date:        Mon Aug 16 12:35:04 2010 -0700
  summary:     Bug 581263 - remove slow natives (r=waldo,mrbkap)
Assignee

Comment 1

9 years ago
The problem is that native constructor calls no longer replace primitive return values with the original constructed 'this' -- there is no original constructed this.  So it falls on proxy_Construct to do the right thing, which I missed in the rm-slow-natives patch.

In doing the obvious thing (using js_NewInstance, which gives you the same 'this' object you got with slow native constructors), I had a question: was the old behavior the intended behavior? That is, since the callee is a Proxy, the default 'this' return value (e.g., in the testcase in comment 0) will be a Proxy object.  I don't know anything about proxies, but I would have guessed you would want the return value, if the handler returns a primitive, to be (new Object()).

Andreas or Brendan: any thoughts?  I didn't see any obvious answers on the Harmony Proxies page.
Assignee

Comment 2

9 years ago
Ah, Brendan said that this was discussed recently in committee and that proxies are indeed allowed to return primitives.  So, to keep the assertion (which I think is valuable), we can just change the assertion to an implication:

  JS_ASSERT_IF(native != proxy_Construct, !vp->isPrimitive());
Assignee

Comment 4

9 years ago
Posted patch weaken asserts (obsolete) — Splinter Review
This weakens the asserts to give proxies a pass.  This also fixes bug 593473.
Assignee: general → lw
Status: NEW → ASSIGNED
Attachment #472064 - Flags: review?(brendan)
Assignee

Updated

9 years ago
Duplicate of this bug: 593473
Assignee

Comment 6

9 years ago
Posted patch fixSplinter Review
mrbkap was kind enough to point out that my previous patch commented out most of test_wrappers.xul :P
Attachment #472064 - Attachment is obsolete: true
Attachment #472732 - Flags: review?(brendan)
Attachment #472064 - Flags: review?(brendan)
Attachment #472732 - Flags: review?(brendan) → review+
Assignee

Updated

9 years ago
Duplicate of this bug: 594656
Assignee

Comment 8

9 years ago
http://hg.mozilla.org/tracemonkey/rev/358b99602aea
Whiteboard: fixed-in-tracemonkey

Updated

9 years ago
blocking2.0: --- → betaN+

Comment 9

9 years ago
http://hg.mozilla.org/mozilla-central/rev/358b99602aea
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.