Closed Bug 1008818 Opened 6 years ago Closed 6 years ago

OdinMonkey: Add implicitly inherited "use strict" directive in asm.js functions source

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: gkw, Assigned: bbouvier)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files)

x = Object
try {
    (function() {
        "use strict";
        eval("\
            Object.toSource = (function() {\
                \"use asm\";\
                function f() {}\
                return f\
            })\
        ")
    })()
} catch (e) {}
print(uneval(x))

$ ./js-dbg-32-dm-ts-darwin-20ca234fd62b --fuzzing-safe --ion-parallel-compile=off 9088.js
function f() {}

$ ./js-dbg-32-dm-ts-darwin-20ca234fd62b --fuzzing-safe --ion-parallel-compile=off --no-fpu 9088.js
function f() {
"use strict";
}

(Tested this on 32-bit Mac js opt threadsafe deterministic shell off m-c rev 20ca234fd62b)

My configure flags are:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>


This seems to go way back to rev 37e29c27e6e8, however, Benjamin, do you mind looking at this?
Flags: needinfo?(benj)
Nice! asm.js functions can also inherit the strict context, as this test case shows.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Flags: needinfo?(benj)
Attached patch Patch + testSplinter Review
This is basically a copy and paste of the AsmJSModuleToString code for implicit strict functions, with the difference that for asm.js functions, the source also contains the function name. Refactoring will come afterwards, for less cognitive load.
Attachment #8421081 - Flags: review?(luke)
Attached patch RefactoringSplinter Review
Tidy up the two AsmJS*ToString functions.
Attachment #8421082 - Flags: review?(luke)
Comment on attachment 8421081 [details] [diff] [review]
Patch + test

Thanks!
Attachment #8421081 - Flags: review?(luke) → review+
Comment on attachment 8421082 [details] [diff] [review]
Refactoring

Nice!
Attachment #8421082 - Flags: review?(luke) → review+
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b755482b48c9
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/438e99b1cf5a
Summary: OdinMonkey: Differential Testing: Different output message involving "use strict" → OdinMonkey: Add implicitly inherited "use strict" directive in asm.js functions source
https://hg.mozilla.org/mozilla-central/rev/b755482b48c9
https://hg.mozilla.org/mozilla-central/rev/438e99b1cf5a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.