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)
Tracking
()
RESOLVED
FIXED
mozilla32
Tracking | Status | |
---|---|---|
firefox32 | --- | fixed |
People
(Reporter: gkw, Assigned: bbouvier)
References
Details
(Keywords: regression, testcase)
Attachments
(1 file)
3.02 KB,
patch
|
mjrosenb
:
review+
|
Details | Diff | Splinter Review |
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)
![]() |
Reporter | |
Updated•11 years ago
|
Summary: Differential Testing: Different output message involving Math.sin and Math.ceil → Differential Testing: Different output message involving Math.ceil
Assignee | ||
Comment 1•11 years ago
|
||
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)
Comment 2•11 years ago
|
||
(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)
Assignee | ||
Comment 3•11 years ago
|
||
(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)
Updated•11 years ago
|
Attachment #8428725 -
Flags: review?(mrosenberg) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
![]() |
Reporter | |
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•