Closed Bug 942549 Opened 7 years ago Closed 7 years ago

Differential Testing: Different output message involving eval, -0

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox28 --- fixed

People

(Reporter: gkw, Assigned: shu)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file, 1 obsolete file)

function m(f, inputs) {
    results = []; {
        for (var j = 0; j < inputs.length; ++j) {
            for (var k = 0; k < inputs.length; ++k) {
                try {
                    results.push(f())
                } catch (e) {}
            }
        }
    }
    print((uneval(results)))
}
function g() {
    return eval(-0)
}
m(g, [, 0])

shows [-0, -0, -0, -0] with --fuzzing-safe only but shows [-0, -0, 0, 0] with --fuzzing-safe --ion-eager, on m-c changeset ad6589ed742c on a 32-bit opt deterministic shell. 

My configure flags are:

CC="gcc -m32" AR=ar PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig CXX="g++ -m32" sh ./configure --target=i686-pc-linux --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-elf-hack --enable-stdcxx-compat --enable-warnings-as-errors --enable-signmar --disable-elf-hack --enable-js-diagnostics --with-intl-api=build --enable-ctypes --disable-shared-js --enable-jemalloc --with-ccache --enable-threadsafe <other NSPR options>

Due to skipped revisions, the first bad revision could be any of:
changeset:   http://hg.mozilla.org/mozilla-central/rev/12e5bcffbbac
user:        Shu-yu Guo
date:        Thu Jun 27 14:47:44 2013 -0700
summary:     Bug 877893 - Part 3: VM & Ion changes to make MToString threadsafe. (r=billm)

changeset:   http://hg.mozilla.org/mozilla-central/rev/611624b7ddb4
user:        Shu-yu Guo
date:        Thu Jun 27 14:47:44 2013 -0700
summary:     Bug 877893 - Part 4: Make MToString support doubles and threadsafe. (r=djvj)

Shu-yu, is bug 877893 a possible regressor?
Flags: needinfo?(shu)
Severity: critical → major
This is an existing bug in Ion. Direct eval is supposed to act as identity on non-strings, but Ion always converts its argument to string. My patch surfaced this since I expanded the ToString logic to include numbers.

Patch forthcoming.
Flags: needinfo?(shu)
Attachment #8341353 - Flags: review?(jdemooij)
Assignee: general → shu
Status: NEW → ASSIGNED
Comment on attachment 8341353 [details] [diff] [review]
Fix direct eval on non-strings in Ion.

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

Nice find!
Attachment #8341353 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/3d670ed9debf
https://hg.mozilla.org/mozilla-central/rev/aa65865e4f66
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Attached patch Fix linker errors in patch for (obsolete) — Splinter Review
The patch for bug 942549 introduced const static integral
members that do not have a definition and are used where an
integral constant expression is not required.
Alo!

I just pulled and got undefined reference linker errors with this patch. Seems like a few const static members are not defined. I always thought you could just use a member static const int foo = blah; as long as you include the .h file. But the standard seems kind of unclear (unless someone can clear it up for me). It looks like you can't use Class::foo anywhere except where a constant expression is required (officially, but some compilers seem happy)...

Created bug 947235
Oops, I seem to have attached patch to this bug by accident, was meant to go to the bug. Apologies I'm very new to the whole process here and I'm not even sure if this requires a whole new bug, or not. And I'm a bit surprised with the linker errors as well :)
Comment on attachment 8343756 [details] [diff] [review]
Fix linker errors in patch for

(patch was for wrong bug)
Attachment #8343756 - Attachment is obsolete: true
This is breaking at least some Linux builds (though not on Tinderbox).  See bug 947642.
Depends on: 947642
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.