Closed Bug 1015656 Opened 11 years ago Closed 11 years ago

Differential Testing: Different output message involving Math.ceil

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
All
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox32 --- fixed

People

(Reporter: gkw, Assigned: bbouvier)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file)

function f(x, y) { return Math.sin(y) < Math.ceil(x) } f(0.1, 1) print(f(4294967296, 1)) $ ./js-opt-32-dm-ts-armSim-darwin-e86a0d92d174 --fuzzing-safe --ion-parallel-compile=off testcase.js true $ ./js-opt-32-dm-ts-armSim-darwin-e86a0d92d174 --fuzzing-safe --ion-parallel-compile=off --ion-eager testcase.js false (Tested this on 32-bit Mac ARM simulator js opt threadsafe deterministic shell off m-c rev e86a0d92d174) 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-arm-simulator --enable-optimize --disable-debug --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: http://hg.mozilla.org/mozilla-central/rev/84e12d8fa8d5 user: Benjamin Bouvier date: Thu May 22 12:03:08 2014 +0200 summary: Bug 1010747: Implement Ceil (floating-point) -> int32 in Ion; r=sunfish,mjrosenb Benjamin, is bug 1010747 a likely regressor?
Flags: needinfo?(benj)
Summary: Differential Testing: Different output message involving Math.sin and Math.ceil → Differential Testing: Different output message involving Math.ceil
Attached patch Patch + testSplinter Review
If the input value is strictly above UINT_MAX, it seems that the f64 -> u32 conversion returns -1 (and then by adding 1 as -1 !== input, ceil yields 0). Not sure if that is specific to the ARM simulator or if it is specified in the architecture? (couldn't find it in the ARM ARM i have). https://tbpl.mozilla.org/?tree=Try&rev=8d0db5b563e4
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8428725 - Flags: review?(mrosenberg)
Flags: needinfo?(benj)
(In reply to Benjamin Bouvier [:bbouvier] from comment #1) > Created attachment 8428725 [details] [diff] [review] > Patch + test > > If the input value is strictly above UINT_MAX, it seems that the f64 -> u32 > conversion returns -1 (and then by adding 1 as -1 !== input, ceil yields 0). > Not sure if that is specific to the ARM simulator or if it is specified in > the architecture? (couldn't find it in the ARM ARM i have). That sounds like how the ARM hardware behaves. Does this patch fail/misbehave because of this? > > https://tbpl.mozilla.org/?tree=Try&rev=8d0db5b563e4
Flags: needinfo?(benj)
(In reply to Marty Rosenberg [:mjrosenb] from comment #2) > (In reply to Benjamin Bouvier [:bbouvier] from comment #1) > > Created attachment 8428725 [details] [diff] [review] > > Patch + test > > > > If the input value is strictly above UINT_MAX, it seems that the f64 -> u32 > > conversion returns -1 (and then by adding 1 as -1 !== input, ceil yields 0). > > Not sure if that is specific to the ARM simulator or if it is specified in > > the architecture? (couldn't find it in the ARM ARM i have). > That sounds like how the ARM hardware behaves. Does this patch > fail/misbehave because of this? The previous ceil implementation was misbehaving because of that indeed. The patch attached to this bug fixes the issue by bailing out on 0, (i.e. -1 for the bad conversion plus 1 as input and ScratchFloatReg are different), for non negative numbers (which is sane -- Math.ceil(x) can't be 0 if x > 0).
Flags: needinfo?(benj)
Attachment #8428725 - Flags: review?(mrosenberg) → review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: