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)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: gkw, Assigned: bbouvier)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file)

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)
Oh, hah, the toSource() inject "use strict" when a function is defined inside a "use strict" context.
Summary: OdinMonkey: Differential Testing: Different output message involving Math.round and Math.tan → OdinMonkey: Differential Testing: Different output message involving "use strict"
Attached patch Patch + testSplinter Review
Quite a journey. Nice catch!
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8407531 - Flags: review?(luke)
Flags: needinfo?(benj)
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+
(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
https://hg.mozilla.org/mozilla-central/rev/a93e17835a75
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
QA Whiteboard: [qa-]
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.