Closed Bug 912316 Opened 6 years ago Closed 6 years ago

Differential Testing: Different output message involving Proxy and __proto__

Categories

(Core :: JavaScript Engine, defect, major)

x86
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla26

People

(Reporter: gkw, Assigned: efaust)

References

(Blocks 2 open bugs)

Details

(Keywords: testcase)

Attachments

(1 file)

count = 0;
try {
    f2 = (function(a, a1, a) {
        count++;
        x = 1 % print('foo' + count);
        var r = a1 / 4
    });
    i1 = Iterator(1, true)
    a0 = []
    this.f2()
    e2 = x
    Object.defineProperty(a0, 9, {
        value: this.e2
    })
    i1.toSource = (function() {
        Array.prototype.sort.call(a0, (function() {}))
    })
    Object.defineProperty(a0, 15, {
        writable: 7,
    })
    Array.prototype.push.call(a0, i1)
    v = (this.v instanceof i1)
} catch (e) {}
try {
    Array.prototype.reverse.call(a0);
} catch (e) {}
try {
    i1.valueOf = (function() {
        f2(4 == 0)
    });
    Array(a0());
} catch (e) {}
try {
    Array.prototype.sort.call(a0, f2)
} catch (e) {}

prints "foo" 9 times on js opt shell on m-c changeset e3785e299ab6 without any CLI arguments, when the testcase is passed in as a CLI argument. However it prints "foo" only 8 times with --ion-eager.

My configure flags are:

--enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-threadsafe <more NSPR compilation flags>
Flags: needinfo?(jdemooij)
o1 = new Object
h1 = {}
f1 = Proxy.create(h1)
o2 = {}
v0 = new Number()
try {
    (function() {
        count = 0;
        h1.has = (function() {
            count++;
            print("foo" + count);
        })
    })()
    v0.valueOf = (function() {
        for (var j = 0; j < 9; j++) {
            o1.f()
        }
    })
    o2.__proto__ = f1
    o1.__proto__ = o2; + v0
} catch (e) {}

(this may be a related testcase)

$ js-opt-32-dm-ts-windows-e3785e299ab6.exe --fuzzing-safe 33450.js
foo1
foo2

$ js-opt-32-dm-ts-windows-e3785e299ab6.exe --fuzzing-safe --ion-eager 33450.js
foo1
foo2
foo3
Gary I can't reproduce this with the testcase in comment 0; it only prints "foo1" and "foo2". It *looks* like it's a duplicate of bug 911369, but I can't reproduce it even without the patch for that bug.

I see the problem with the test in comment 1 though, I reduced it to:

o1 = {};
h1 = {};
proxy = Proxy.create(h1);
count = 0;
h1.has = (function() {
    print(count++);
});
function foo() {
    for (var j = 0; j < 9; j++) {
        o1.f();
    }
}
o2 = {};
o2.__proto__ = proxy;
o1.__proto__ = o2;
try {
    foo();
} catch (e) {}

Eric, would you mind taking this? It looks like it's a proxy issue; we're somehow calling h1.has 3 times with --ion-eager instead of twice.
Flags: needinfo?(jdemooij) → needinfo?(efaustbmo)
OK, I have a bead on this. The problem is that we have two stubs that independently call lookupGeneric() when trying to decide if they should attach, so if the first fails, and the second succeeds, we call twice when we should have only called once. Patch hopefully forthcoming.
Assignee: general → efaustbmo
Status: NEW → ASSIGNED
Flags: needinfo?(efaustbmo)
Attached patch FixSplinter Review
OK, so the explanation above was almost right. I can't read GDB backtraces, apparently. The problem was that with the proxy on the proto chain, we weren't ensuring that a lookupProperty() call meant to be idempotent and diagnostic wouldn't call hooks in the proper fashion.

The attached patch moves away from ever calling JSObject::lookupProperty() in this case, and calls LookupPropertyPure() instead.

This also affected GetElementIC in the same way, and this patch also fixes that deficiency.
Attachment #799676 - Flags: review?(kvijayan)
Since the bug morphed to the testcase in comment 1, I'm setting a needinfo on myself to retest the testcase in comment 0 after all these differential testing patches have landed.
Flags: needinfo?(gary)
Summary: Differential Testing: Different output message involving Array.prototype → Differential Testing: Different output message involving Proxy and __proto__
Comment on attachment 799676 [details] [diff] [review]
Fix

Review of attachment 799676 [details] [diff] [review]:
-----------------------------------------------------------------

Nice fix.
Attachment #799676 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/97ee6915d2f4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(In reply to Jan de Mooij [:jandem] from comment #2)
> Gary I can't reproduce this with the testcase in comment 0; it only prints
> "foo1" and "foo2". It *looks* like it's a duplicate of bug 911369, but I
> can't reproduce it even without the patch for that bug.

I've tried to reproduce on m-c rev tip 34f431863037 on Windows 7, it seems to be fixed now. ("FOO" appears 9 times both with and without --ion-eager.) I've verified that the testcase in comment 2 is also fixed.
Status: RESOLVED → VERIFIED
Flags: needinfo?(gary)
You need to log in before you can comment on or make changes to this bug.