Closed
Bug 572983
Opened 14 years ago
Closed 13 years ago
ctypes.Int64(0x7fffffffffffffff) and ctypes.Int64(0x8000000000000000) does not throw type error on SPARC
Categories
(Core :: js-ctypes, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)
Details
Attachments
(1 file, 1 obsolete file)
1.11 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
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?
BTW: dump(0x80000000 << 0) returns -2147483648 on both x86 and SPARC. I think it is defined as ECMAScript Spec 9.5.
Comment 2•14 years ago
|
||
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
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•14 years ago
|
||
OK, then we need a ConvertImpl override for SPARC for floats and doubles. Care to write one? ;)
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.
oops, that is not an expression as I expected, the result should blame dump().
on x86, it returns 0x8000000000000000 for overflow Note: I have to use ">=" because double is not accurate.
fix a typo
Attachment #460837 -
Attachment is obsolete: true
Attachment #460838 -
Flags: review?(dwitte)
Attachment #460837 -
Flags: review?(dwitte)
Comment 9•13 years ago
|
||
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
Attachment #460838 -
Flags: review?(dwitte) → review+
Assignee | ||
Comment 10•13 years ago
|
||
Please land it for me because my hg account doesn't work at this moment.
Assignee | ||
Comment 11•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/7653363bd293
Comment 12•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/7653363bd293
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in
before you can comment on or make changes to this bug.
Description
•