IonMonkey: Assertion failure: obj, at ../../jsval.h:534

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jandem, Assigned: dvander)

Tracking

(Blocks: 2 bugs)

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
The following testcase crashes with --ion-eager -n:
--
delete String.prototype.indexOf;

function enterFunc (funcName) {
    funcName.indexOf();
}
enterFunc(new Array("foo"));
enterFunc(new String("Foo"));
--
The problem is that the GetPropertyCacheT for funcName.indexOf has known "object" return type, but when enterFunc is called again the return type becomes "undefined" and the script is invalidated by the Monitor call. However, the bailout mechanism still thinks the return type must be "object" so we try to construct an invalid Value with "object" type tag and "undefined" payload.

Noticed this when investigating bug 728187.
Yuck. The problem is that GetPropertyCacheT can actually return a Value, we've just made the unbox operation implicit. I think the easiest way to fix this is to get rid of GetPropertyCacheT, and always have explicit unboxing. Then we can safely bail out in between the getprop and the unbox.

Brian, does that sound okay?
Created attachment 598472 [details] [diff] [review]
potential fix

Not sure if this is the right thing yet, but fixes the bug.
Assignee: general → dvander
Status: NEW → ASSIGNED
Group: mozilla-confidential
Group: mozilla-confidential
Created attachment 599835 [details] [diff] [review]
alternate fix

This patch adds more code and is grosser than I hoped, since I had to work around various asserts. I'm not against this facility but it would be nice if we didn't need it.

Brian, how much do you think preserving GetPropertyCacheT will matter? Could we get a microbenchmark that has a lot of register pressure?
Something like this should have more register pressure (assuming m/n/p are hoisted into registers):

function foo(x, m, n, p) {
  var res = 0;
  for (var i = 0; i < m; i++)
    res += n + p + x.f + x.g;
  return res;
}
for (var i = 0; i < 10000; i++) {
  x = {};
  x.f = 1;
  x.g = 1;
  foo(x, 10000, 1, 1);
}
before --ion -n: 458ms
          -m -n: 349ms
after  --ion -n: 490ms 

About a 6% difference, works for me!
Attachment #599835 - Flags: review?(bhackett1024)
Attachment #599835 - Flags: review?(bhackett1024) → review+
Duplicate of this bug: 729808
Duplicate of this bug: 729809
Duplicate of this bug: 729812
Duplicate of this bug: 729895
http://hg.mozilla.org/projects/ionmonkey/rev/418840ca313e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.