781364 - IonMonkey: Crash [@ js::ShapeTable::search] or "Assertion failure: table_ && isOwned(),"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

k = m
function h() {
  switch (true) {
  default:
    x = newGlobal('')
  }
  return function(f, code) {
    try {
      evalcx(code, x)
    } catch (e) {}
  }
}
function m() {
  f()
}
function g(code) {
  f = new Function(code);
  k(f, code)
}
g("k=h()")
g("\
    a='';\
    Object.defineProperty(this,\"b\",{get:function(){a=this.d()}});\
    Object.defineProperty(this,\"c\",{get:function(){b}});\
    d=2;\
    b\
");
g("b");
g("b");
g("for(v of c);");
g("a=eval(\"function f(){}\");b")

asserts js debug shell on IonMonkey changeset 21b4797e4cb3 with --ion-eager and -a at Assertion failure: table_ && isOwned(), and also crashes js opt shell at js::ShapeTable::search when the testcase is passed in as a CLI argument.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   101428:4d18d3c3f50f
user:        Jan de Mooij
date:        Thu Jul 19 10:02:36 2012 +0200
summary:     Bug 771130 part 2 - Inline monomorphic GETPROP and SETPROP. r=dvander

Comment 1

[Eric Faust [:efaust]]

Attachment: Fix

It is a violation of the calling contract to call Shape::search() with a shape that is not the start of a shape chain.

Comment 2

[Eric Faust [:efaust]]

Attachment: Suboptimal Fix

The immediate problem was that it is invalid to call Shape::search() on shapes in dictionary mode if the shape you are calling it on isn't the lastProperty() of the relevant object. Since the shape comes out of the PIC, we cannot guarantee that.

More broadly, though, it's downright silly to take this codepath in that case. The shape lookup path may do allocations and optimizations to speed up future accesses (on a shape that isn't normally accessed directly through), and worse, we generate a shape guard based on this shape that can *never* succeed if it's not a lastProperty().

We ought to come up with some better way, but at least this won't crash.

Comment 3

[Eric Faust [:efaust]]

https://hg.mozilla.org/projects/ionmonkey/rev/d2a2fb2cc563

Comment 4

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397