Closed Bug 1536228 Opened 9 months ago Closed 9 months ago

15.2.3.6-dictionary-redefinition-16-of-32.js fails with Ion enabled

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(2 files)

Running jstests.py --run-slow-tests dist/bin/js non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js fails with:

## non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js: rc = 3, run time = 8.237467
560566: ES5 Object.defineProperty(O, P, Attributes): dictionary redefinition 16 of 32
Running dictionary already-present tests now...
Starting test with old descriptor ({value:(void 0), configurable:true, writable:false})...
Starting test with old descriptor ({value:(void 0), configurable:true})...
Starting test with old descriptor ({value:(void 0), configurable:false, writable:true})...
Starting test with old descriptor ({value:(void 0), configurable:false, writable:false})...
Starting test with old descriptor ({value:(void 0), configurable:false})...
Starting test with old descriptor ({value:(void 0), writable:true})...
Starting test with old descriptor ({value:(void 0), writable:false})...
Starting test with old descriptor ({value:(void 0)})...
Starting test with old descriptor ({value:true, enumerable:true, configurable:true, writable:true})...
Starting test with old descriptor ({value:true, enumerable:true, configurable:true, writable:false})...
Starting test with old descriptor ({value:true, enumerable:true, configurable:true})...
Starting test with old descriptor ({value:true, enumerable:true, configurable:false, writable:true})...
Difference when comparing native/reimplementation behavior for readded descriptor ({value:-Infinity, enumerable:true, configurable:true, writable:true}), initial was ({value:true, enumerable:true, configurable:false, writable:true}); native readd didn't throw, reimpl add did, error: TypeError: can't make configurable again; rejected!
Starting test with old descriptor ({value:true, enumerable:true, configurable:false, writable:false})...
Starting test with old descriptor ({value:true, enumerable:true, configurable:false})...
Starting test with old descriptor ({value:true, enumerable:true, writable:true})...
Starting test with old descriptor ({value:true, enumerable:true, writable:false})...
Starting test with old descriptor ({value:true, enumerable:true})...
Starting test with old descriptor ({value:true, enumerable:false, configurable:true, writable:true})...
Starting test with old descriptor ({value:true, enumerable:false, configurable:true, writable:false})...
Starting test with old descriptor ({value:true, enumerable:false, configurable:true})...
Starting test with old descriptor ({value:true, enumerable:false, configurable:false, writable:true})...
Dictionary property-present fraction 16 of 32 completed!
Full accumulated number of errors: 1
uncaught exception: Error thrown during testing: 1 errors detected, FAIL at line undefined

(Unable to print stack trace)
REGRESSION - non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js
[0|1|0|0] 100% ==========================================================>|   8.8s
REGRESSIONS
    non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js

Running with --tbpl shows that the failure is somehow Ion-related, because only the "default" and "baseline-eager" configuration are affected, but not for example "baseline-only" or "interpreter-only":

REGRESSIONS
    non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js
    --baseline-eager non262/object/15.2.3.6-dictionary-redefinition-16-of-32.js

Hi Jan, I've got a quick question. This bug is caused by SetPropIRGenerator attaching the MegamorphicStoreSlot here. What do you think is more useful: Adding a new MegamorphicInitSlot to CacheIRCompiler, which basically works like the existing CacheIRCompiler::emitMegamorphicStoreSlot method, but additionally checks for non-configurable and non-enumerable properties here. Or alternatively prevent CodeGenerator to use CacheKind::SetProp here for JOF_PROPINIT operations?

Flags: needinfo?(jdemooij)

Reduced test case:

function f(x) {
    var n = 5000;
    for (var i = 0; i < n; ++i) {
        var obj = {};

        Object.defineProperty(obj, "bar" + i, {value:0, enumerable:false, configurable:true, writable:true});

        Object.defineProperty(obj, "foo", {value:0, enumerable:true, configurable:true, writable:true});
        Object.defineProperty(obj, "foo", {value:0, enumerable:true, configurable:!(x && i + 1 === n), writable:true});
        Object.defineProperty(obj, "foo", {value:0, enumerable:true, configurable:true, writable:true});
    }
}

var n = 2;
for (var i = 0; i < n; ++i) {
    try {
        f(i + 1 === n);
    } catch (e) {
        if (e instanceof TypeError) {
            continue;
        }
    }
    if (i + 1 === n) {
        assertEq(true, false, "missing or wrong error");
    }
}

(In reply to André Bargull [:anba] from comment #1)

Hi Jan, I've got a quick question. This bug is caused by SetPropIRGenerator attaching the MegamorphicStoreSlot here.

Sorry for the delay. Would it work to check IsPropertySetOp(*pc) there?

Flags: needinfo?(jdemooij)

Yes, I guess that should work, too.

Assignee: nobody → andrebargull
Status: NEW → ASSIGNED

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0afcf25330d9
Part 1: Fix typos and remove unnecessary blocks. r=jandem
https://hg.mozilla.org/integration/autoland/rev/d3e9985fd713
Part 2: Don't emit megamorphic store slot stub for JSOP_INITELEM. r=jandem

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.