Closed Bug 1423139 Opened 4 years ago Closed 4 years ago

Differential Testing: Different output message involving Object.defineProperty

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
All
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: gkw, Assigned: jandem)

References

Details

(Keywords: testcase)

Attachments

(1 file)

x = {};
x.set = function() {};
x.has = function() {};
for (var p in x) {
    try {
        Object.defineProperty([], "length", {
            configurable: 1,
            enumerable: 1,
            writable: 1,
            value: 0
        });
    } catch (e) {
        print(e);
    }
}


$ ./js-64-dm-linux-e19b017880c8 --fuzzing-safe --no-threads --baseline-eager --no-ion testcase.js
TypeError: can't redefine non-configurable property "length"

$ ./js-64-dm-linux-e19b017880c8 --fuzzing-safe --no-threads --no-baseline --no-ion testcase.js
TypeError: can't redefine non-configurable property "length"
TypeError: can't redefine non-configurable property "length"

Tested this on m-c rev e19b017880c8.

My configure flags are:

AR=ar sh ./configure --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u -m funfuzz.js.compile_shell -b "--enable-more-deterministic" -r e19b017880c8

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a14fc8f5babc
user:        André Bargull
date:        Mon Aug 28 14:19:34 2017 +0200
summary:     Bug 1303335: Move parts of Object.getOwnProperty and Object.defineProperty to self-hosted code. r=till

Andre, is bug 1303335 a likely regressor?
Flags: needinfo?(andrebargull)
Object.defineProperty(...) calls _DefineDataProperty [1] and _DefineDataProperty compiles down to JSOP_INITELEM [2]. But when we compile JSOP_INITELEM, we use the SetPropIR generator [3], so we end up with generating a normal set-stub for array length [4], which is obviously wrong in this case.

:jandem, can you take this issue?

[1] https://github.com/mozilla/gecko-dev/blob/2e08acdf8862e68b13166970e17809a3b5d6a555/js/src/builtin/Object.js#L267
[2] https://github.com/mozilla/gecko-dev/blob/2e08acdf8862e68b13166970e17809a3b5d6a555/js/src/frontend/BytecodeEmitter.cpp#L9075
[3] https://github.com/mozilla/gecko-dev/blob/2e08acdf8862e68b13166970e17809a3b5d6a555/js/src/jit/BaselineIC.cpp#L1023-L1025
[4] https://github.com/mozilla/gecko-dev/blob/2e08acdf8862e68b13166970e17809a3b5d6a555/js/src/jit/CacheIR.cpp#L2855
Flags: needinfo?(andrebargull) → needinfo?(jdemooij)
Attached patch PatchSplinter Review
Thanks for the analysis, André.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8934535 - Flags: review?(evilpies)
Attachment #8934535 - Flags: review?(evilpies) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/703171a447be
Don't attach a SetArrayLength stub for JSOP_INITELEM. r=evilpie
https://hg.mozilla.org/mozilla-central/rev/703171a447be
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.