Closed Bug 728188 Opened 8 years ago Closed 8 years ago

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


(Core :: JavaScript Engine, defect)

Not set





(Reporter: jandem, Assigned: dvander)


(Blocks 1 open bug)



(2 files)

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

function enterFunc (funcName) {
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?
Attached patch potential fixSplinter Review
Not sure if this is the right thing yet, but fixes the bug.
Assignee: general → dvander
Group: mozilla-confidential
Group: mozilla-confidential
Attached patch alternate fixSplinter Review
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
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.