ctypes.Int64(0x7fffffffffffffff) and ctypes.Int64(0x8000000000000000) does not throw type error on SPARC

RESOLVED FIXED in mozilla9

Status

()

Core
js-ctypes
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Ginn Chen, Assigned: Ginn Chen)

Tracking

Trunk
mozilla9
Sun
OpenSolaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.11 KB, patch
jorendorff
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
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?
(Assignee)

Comment 1

7 years ago
BTW: dump(0x80000000 << 0) returns -2147483648 on both x86 and SPARC.
I think it is defined as ECMAScript Spec 9.5.

Comment 2

7 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
(Assignee)

Comment 3

7 years ago
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

7 years ago
OK, then we need a ConvertImpl override for SPARC for floats and doubles. Care to write one? ;)
(Assignee)

Comment 5

7 years ago
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.
(Assignee)

Comment 6

7 years ago
oops, that is not an expression as I expected, the result should blame dump().
(Assignee)

Comment 7

7 years ago
Created attachment 460837 [details] [diff] [review]
patch

on x86, it returns 0x8000000000000000 for overflow
Note: I have to use ">=" because double is not accurate.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #460837 - Flags: review?(dwitte)
(Assignee)

Comment 8

7 years ago
Created attachment 460838 [details] [diff] [review]
patch

fix a typo
Attachment #460837 - Attachment is obsolete: true
Attachment #460838 - Flags: review?(dwitte)
Attachment #460837 - Flags: review?(dwitte)
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

6 years ago
Please land it for me because my hg account doesn't work at this moment.
(Assignee)

Comment 11

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/7653363bd293
http://hg.mozilla.org/mozilla-central/rev/7653363bd293
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.