Closed Bug 1033873 Opened 5 years ago Closed 5 years ago

Differential Testing: Different output message involving __proto__

Categories

(Core :: JavaScript Engine: JIT, defect, major)

x86_64
All
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox33 --- fixed
firefox34 --- fixed

People

(Reporter: gkw, Assigned: efaust)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, testcase)

Attachments

(2 files)

function f() {
    return __proto__
}
f()
f()
print(f())


$ ./js-dbgDisabled-opt-64-prof-dm-ts-darwin-5d9af625f42e --fuzzing-safe --ion-offthread-compile=off testcase.js
[object Object]

$ ./js-dbgDisabled-opt-64-prof-dm-ts-darwin-5d9af625f42e --fuzzing-safe --ion-offthread-compile=off --ion-eager testcase.js
null

(Tested this on 64-bit Mac js opt threadsafe deterministic shell off m-c rev 5d9af625f42e)

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --disable-debug --enable-optimize --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/157947d5fe31
user:        Eric Faust
date:        Wed Jun 18 13:00:17 2014 -0700
summary:     Bug 1022736 - Allow cacheing of GlobalObject getters on non-global scopechains. (r=jandem)

Brian, is bug 1022736 a likely regressor?
Flags: needinfo?(efaustbmo)
Oh yeah. This one's definitely on me. Taking.
Assignee: nobody → efaustbmo
Flags: needinfo?(efaustbmo)
Attached patch FixSplinter Review
Clean up some scratch register aliasing issues.
Attachment #8449962 - Flags: review?(jdemooij)
Comment on attachment 8449962 [details] [diff] [review]
Fix

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

Please also add a testcase that fails without this patch. r=me with that.

::: js/src/jit/IonCaches.cpp
@@ +1004,5 @@
>                     ImmGCPtr(holder->lastProperty()),
>                     failures);
>  
> +    if (restoreObjReg)
> +        masm.pop(object);

NameIC::attachCallGetter has:

Register scratchReg = outputReg().valueReg().scratchReg();

And passes that as the object register. So we were always passing the "holder" instead of "object" to EmitGetterCall right? Good catch, fuzzing FTW.
Attachment #8449962 - Flags: review?(jdemooij) → review+
Nits addressed. Carrying r+.
Attachment #8460764 - Flags: review+
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/73b13f03a16b
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: needinfo?(cbook)
QA Whiteboard: [qa-]
Depends on: 1046597
You need to log in before you can comment on or make changes to this bug.