Closed
Bug 996881
Opened 10 years ago
Closed 10 years ago
OdinMonkey: Differential Testing: Different output message involving "use strict"
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: gkw, Assigned: bbouvier)
References
Details
(Keywords: regression, testcase)
Attachments
(1 file)
9.19 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
print(eval("\ \"use strict\";\ (function(stdlib, n, heap) {\ \"use asm\";\ function f() {}\ return f\ })")) $ ./js-opt-64-dm-ts-darwin-265d82091bce --fuzzing-safe --ion-parallel-compile=off 8233.js function (stdlib, n, heap) { "use asm"; function f() {} return f} $ ./js-opt-64-dm-ts-darwin-265d82091bce --fuzzing-safe --no-asmjs --baseline-eager --ion-parallel-compile=off --no-ion 8233.js function (stdlib, n, heap) { "use strict"; "use asm"; function f() {} return f} (Tested this on 64-bit Mac js opt threadsafe deterministic shell off m-c rev 265d82091bce, and I think it also reproduces on Linux) My configure flags (Mac) 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 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options> This seems to go back further than http://hg.mozilla.org/mozilla-central/rev/541248fb29e4, but I'm not sure. Benjamin, is this related to bug 878399?
Flags: needinfo?(benj)
Comment 1•10 years ago
|
||
Oh, hah, the toSource() inject "use strict" when a function is defined inside a "use strict" context.
Reporter | ||
Updated•10 years ago
|
Summary: OdinMonkey: Differential Testing: Different output message involving Math.round and Math.tan → OdinMonkey: Differential Testing: Different output message involving "use strict"
Assignee | ||
Comment 2•10 years ago
|
||
Quite a journey. Nice catch!
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8407531 -
Flags: review?(luke)
Flags: needinfo?(benj)
Comment 3•10 years ago
|
||
Comment on attachment 8407531 [details] [diff] [review] Patch + test Review of attachment 8407531 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/AsmJSLink.cpp @@ +800,5 @@ > if (!src) > return nullptr; > > + bool needUseStrict = module.addUseStrictToSource(); > + if (needUseStrict) { if (module.strict()) { ::: js/src/jit/AsmJSModule.h @@ +430,5 @@ > > struct Pod { > uint32_t funcLength_; > uint32_t funcLengthWithRightBrace_; > + bool addUseStrictToSource_; How about just calling it 'strict_'? @@ +512,5 @@ > } > uint32_t offsetToEndOfUseAsm() const { > return offsetToEndOfUseAsm_; > } > + void initFuncEnd(uint32_t endBeforeCurly, uint32_t endAfterCurly, bool needAddUseStrict) { Instead of initializing this at the end of module compilation, could you make this a constructor parameter to AsmJSModule since it's something we know when we begin compilation?
Attachment #8407531 -
Flags: review?(luke) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #3) > Comment on attachment 8407531 [details] [diff] [review] > Patch + test > > Review of attachment 8407531 [details] [diff] [review]: > ----------------------------------------------------------------- > Instead of initializing this at the end of module compilation, could you > make this a constructor parameter to AsmJSModule since it's something we > know when we begin compilation? Fixed, makes sense. https://hg.mozilla.org/integration/mozilla-inbound/rev/a93e17835a75
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a93e17835a75
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•