Last Comment Bug 572983 - ctypes.Int64(0x7fffffffffffffff) and ctypes.Int64(0x8000000000000000) does not throw type error on SPARC
: ctypes.Int64(0x7fffffffffffffff) and ctypes.Int64(0x8000000000000000) does no...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: js-ctypes (show other bugs)
: Trunk
: Sun OpenSolaris
: -- normal (vote)
: mozilla9
Assigned To: Ginn Chen
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-18 00:08 PDT by Ginn Chen
Modified: 2011-08-31 01:55 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.11 KB, patch)
2010-07-28 04:43 PDT, Ginn Chen
no flags Details | Diff | Splinter Review
patch (1.11 KB, patch)
2010-07-28 04:46 PDT, Ginn Chen
jorendorff: review+
Details | Diff | Splinter Review

Description Ginn Chen 2010-06-18 00:08:37 PDT
Caught be xpcshell-tests.

On x86,
ctypes.Int64(0x7fffffffffffffff) and ctypes.Int64(0x8000000000000000) throw TypeError.

(Although it looks like ctypes.Int64(0x7fffffffffffffff) is legal, casting to jsdouble makes it no different to  ctypes.Int64(0x8000000000000000)).

I'm not sure if it is a defined behavior in js-ctypes.

On SPARC, it returns 0x7fffffffffffffff for overflow.

Similarly,
dump(ctypes.int32_t(0x80000000)) prints ctypes.int32_t(-2147483648) on x86 but
prints ctypes.int32_t(2147483647) on SPARC.

Is it a undefined behavior or should it follow ECMAScript Spec 9.5?
Comment 1 Ginn Chen 2010-06-18 00:57:15 PDT
BTW: dump(0x80000000 << 0) returns -2147483648 on both x86 and SPARC.
I think it is defined as ECMAScript Spec 9.5.
Comment 2 dwitte@gmail.com 2010-07-20 10:11:02 PDT
I bet what's happening is that int32_t(2147483648.d) is not returning -2147483648, but the compiler decides to clamp it to the max representable int32_t which is 2147483647.

If someone can roll a trivial .c file to show this is the case, then we can just override ConvertImpl<> like we do for MSVC.

http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#981
Comment 3 Ginn Chen 2010-07-20 11:16:19 PDT
Yes, it is the case on SPARC.

The definition of the SPARC fdtoi assembly is

F[sdq]TOi: When a NaN, infinity, large positive argument ≥ 2147483648.0, or large negative argument ≤ –2147483649.0 is converted to an integer, the invalid_current (nvc) bit of FSR.cexc should be set and fp_exception_ieee_754 should be raised.If the floating-point invalid trap is disabled (FSR.TEM.NVM = 0), no trap occurs and a numerical result is generated: if the sign bit of the operand is 0, the result is 2147483647; if the sign bit of the operand is 1, the result is –2147483648.
Comment 4 dwitte@gmail.com 2010-07-20 12:14:29 PDT
OK, then we need a ConvertImpl override for SPARC for floats and doubles. Care to write one? ;)
Comment 5 Ginn Chen 2010-07-27 05:35:49 PDT
I tried override ConvertImpl, it solved the xpcshell-tests issue.
But I don't know how to solve dump(ctypes.int32_t(0x80000000)).
No matter what I do in ConvertImpl, the result doesn't change.
Comment 6 Ginn Chen 2010-07-27 06:23:54 PDT
oops, that is not an expression as I expected, the result should blame dump().
Comment 7 Ginn Chen 2010-07-28 04:43:11 PDT
Created attachment 460837 [details] [diff] [review]
patch

on x86, it returns 0x8000000000000000 for overflow
Note: I have to use ">=" because double is not accurate.
Comment 8 Ginn Chen 2010-07-28 04:46:46 PDT
Created attachment 460838 [details] [diff] [review]
patch

fix a typo
Comment 9 Jason Orendorff [:jorendorff] 2011-07-27 11:34:20 PDT
Comment on attachment 460838 [details] [diff] [review]
patch

OK, so as I understand it:

  We used code like this to see if a jsdouble's value is exactly that of an
  int64:

    bool isExactInt64(jsdouble d) {
        int64 i = int64(d);
        return jsdouble(i) == d;
    }

  But C++ doesn't guarantee that exact values are the only ones that will
  round-trip. In fact, on some platforms, including SPARC, there are pairs of
  values, a uint64 and a double, such that neither value is exactly
  representable in the other type, but they cast to each other.

Let's put a comment to that effect in the source. It is really the busted code's fault, not SPARC's fault.

r=me.

I can land this if you still want it. Or feel free to land it yourself, but note that the tracemonkey repo is defunct; new changes should be landed in mozilla-inbound:

http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/d6d7a888b0bee104?pli=1
Comment 10 Ginn Chen 2011-07-27 12:12:39 PDT
Please land it for me because my hg account doesn't work at this moment.
Comment 12 Marco Bonardo [::mak] 2011-08-31 01:55:59 PDT
http://hg.mozilla.org/mozilla-central/rev/7653363bd293

Note You need to log in before you can comment on or make changes to this bug.