If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

IonMonkey: IC causes extra Proxy trap

NEW
Unassigned

Status

()

Core
JavaScript Engine
5 years ago
3 years ago

People

(Reporter: djvj, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p3])

(Reporter)

Description

5 years ago
This got picked up by jorendorrf from another bug report on Harmony behaviour here: https://bugzilla.mozilla.org/show_bug.cgi?id=753402

Copy-pasting his comment:

  // |jit-test| mjitalways
  var c = 0;
  var p = Proxy.create({has: function (id) { c++; }});
  var obj = Object.create(p);
  for (var i = 0; i < 4; ++i)
      obj.xyz;
  assertEq(c, 4);

This fails due to some code under GetPropMaybeCached in methodjit/PolyIC.cpp. Toward the bottom of that function there's this:

    if (!monitor.recompiled() && pic->shouldUpdate(f.cx)) {
        GetPropCompiler cc(f, script, obj, *pic, name, stub);
        if (!cc.update())
            THROW();
    }

    Value v;
    if (cached) {
        if (!GetPropertyOperation(f.cx, f.pc(), f.regs.sp[-1], &v))
            THROW();
    } else {
        if (!obj->getProperty(f.cx, name, &v))
            THROW();
    }

The hit trap can be called twice: once from cc.update(), which does a lookup; and then again from obj->getProperty().

I suspect cc.update() should be changed to avoid firing proxy traps, but I can't take this bug.
Whiteboard: [js:p3]
Testcase also fails with --ion-eager, works with --baseline-eager.
Summary: JM: IC causes extra Proxy trap → IonMonkey: IC causes extra Proxy trap
(Assignee)

Updated

3 years ago
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.