Closed Bug 361268 Opened 18 years ago Closed 13 years ago

64 bit operations in libjs depend on hand maintained list of operating systems

Categories

(Core :: JavaScript Engine, defect)

x86
NetBSD
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: pw-fb, Unassigned)

References

Details

User-Agent: Mozilla/5.0 (X11; U; NetBSD i386; en-US; rv:1.9a1) Gecko/20051031 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (X11; U; NetBSD i386; en-US; rv:1.9a1) Gecko/20051031 Firefox/1.6a1 libjs provides "Portable access to 64 bit numerics" via macros in jslong.h which operate on types JS{,U}Int64 and JS{,U}int32 defined in jstypes.h. The definition of JS{,U}Int64 is based on whether or not JS_HAVE_LONG_LONG is defined. JS_HAVE_LONG_LONG is defined in jsosdep.h according to a manually maintained list of operating systems. This list can't be accurate (e.g., Doesn't NetBSD have 64-bit datatypes? What about an ancient copy of said operating system?), and the autoconf philosopy of testing for features rather than OS names seems to be applicable in this case. Before getting out the history books to write an autoconf 2.13 macro to test for long long and define JS_HAVE_LONG_LONG, is that really the right thing to do? To quote mozilla/configure.in: dnl pass -Wno-long-long to the compiler MOZ_ARG_ENABLE_BOOL(long-long-warning, [ --enable-long-long-warning Warn about use of non-ANSI long long type], So, do we really want (quoting jstypes.h) typedef long long JSInt64; typedef unsigned long long JSUint64; ? or rather typedef int64_t JSInt64 typedef int32_t JSInt32 ? so write autoconf 2.13 equivalents of modern day autoconf's -- Macro: AC_TYPE_INT64_T If `stdint.h' or `inttypes.h' defines the type `int64_t', define `HAVE_INT64_T'. Otherwise, define `int64_t' to a signed integer type that is exactly 64 bits wide and that uses two's complement representation, if such a type exists. and -- Macro: AC_TYPE_UINT64_T ? (We could in fact do all the types, so we would't need JS_BYTES_PER_x either, and typedef int16_t JSInt16 etc is quite readable..) Thoughts? Cheers, Patrick Reproducible: Always
(bug born in bug 361075)
Depends on: 104642
*** Bug 361267 has been marked as a duplicate of this bug. ***
spidermonkey doesn't always use autoconf for building, you can also build it with Makefile.ref. That said, it might be better to require a native 64-bit type, like NSPR and Gecko already do...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9?
(In reply to comment #3) > spidermonkey doesn't always use autoconf for building, you can also build it > with Makefile.ref. It seems that you get libjs from Makefile.ref rather than libmozjs - is that intentional? Is there a difference between them? > That said, it might be better to require a native 64-bit type, like NSPR and > Gecko already do... .. at the moment I am autoconfing this, and wondering about word/dword. At least JSWord = sizeof(void*). (and so came across possible trivial bug 363166)
I think bug 97954 or a morphed-to-use-autoconf-only version of it blocks this bug. /be
Depends on: 97954
(In reply to comment #5) > I think bug 97954 or a morphed-to-use-autoconf-only version of it blocks this > bug. I would rather like to think solving this one inadvertently solves that one too.. Any ideas on the libmozjs vs libjs question? (I'm just doing libjs for now..)
(In reply to comment #3) > That said, it might be better to require a native 64-bit type, like NSPR and > Gecko already do... .. back to this part. Do we want to require a native 64-bit type? If that is the case, is saves me having to do the "int64_t" doesn't exist side of configure.ac.
(In reply to comment #6) > (In reply to comment #5) > > I think bug 97954 or a morphed-to-use-autoconf-only version of it blocks this > > bug. > > I would rather like to think solving this one inadvertently solves that one > too.. Think of it however you like, but that bug has the lower number and the wider scope. There's more to unifying build systems than the 64-bit issue. > Any ideas on the libmozjs vs libjs question? (I'm just doing libjs for now..) If you mean this question: > It seems that you get libjs from Makefile.ref rather than libmozjs - is that > intentional? Is there a difference between them? Certainly it's intentional, for historical reasons. libmozjs is built with JS_THREADSAFE defined, while libjs is not. But with a unified build system, while we still could produce both libraries, perhaps we could take a further step and unify the two. Anyway, I think we could unifdef to require long long native support. That would simplify the code quite a bit. That's yet a different bug topic (a bug may already be on file about it), but fixing it would make this bug moot. /be
(In reply to comment #8) > Anyway, I think we could unifdef to require long long native support. That > would simplify the code quite a bit. Also: the only reason we saw the problem in the the non long long case is because jsosdep.h's list was incomplete. If that is replaced by a test, who would actually end up using the typedef struct { #ifdef IS_LITTLE_ENDIAN JSUint32 lo, hi; #else JSUint32 hi, lo; #endif } JSInt64; case? Wouldn't bit-rot set in?
Finally I have managed to autotool js/src, which was rather involved. The distfile created is avaible at http://www.newn.cam.ac.uk/prlw/jsref-1.70.tar.gz Anyway, I have now come to testing, and seem to still have some sort of 32/64 bit bug left behind. Taking this attachment, and doing the traditional gunzip jsref-1.70.tar.gz tar xvf jsref-1.70.tar cd jsref-1.70 ./configure make make check works for NetBSD-4.99.19/i386, but for SunOS-5.9/sun4u/sparc and IRIX-6.5/IP32 make check fails. By hand I see: % ./js js> print(0x123456 * 0x10) 19088736 js> print(0x1234567 * 0x10) 305419888 js> print(0x12345678 * 0x10) and it hangs using cpu. Any thoughts on where to look before I try to debug this on systems I don't own? (BTW --enable-threadsafety is the equivalent of JS_THREADSAFE for the "in-tree" build...)
On the sun: js> print(0x123456 * 0x10) 19088736 js> print(0x1234567 * 0x10) 305419888 js> print(0x12345678 * 0x10) ^C Program received signal SIGINT, Interrupt. 0x9b520 in __muldi3 (u=0x000000001305d130, v=0x000000004cbae924) (gdb) bt #0 0x9b520 in __muldi3 (u=0x000000001305d130, v=0x000000004cbae924) #1 0x34800 in mult (a=0xd337c, b=0xd245c) at jsdtoa.c:688 #2 0x34924 in pow5mult (b=0xd1348, k=43177) at jsdtoa.c:805 #3 0x36e60 in js_dtoa (d=4886718336, mode=0, biasUp=0, ndigits=0, decpt=0xffbfe234, sign=0xffbfe230, rve=0xffbfe22c, buf=0xffbfe2b2 "", bufsize=24) at jsdtoa.c:2518 #4 0x37510 in JS_dtostr (buffer=0xffbfe2b0 "", bufferSize=26, mode=DTOSTR_STANDARD, precision=0, d=4886718336) at jsdtoa.c:2796 #5 0x57e14 in js_NumberToString (cx=0xbb998, d=4886718336) at jsnum.c:717 #6 0x86f10 in js_ValueToString (cx=0xbb998, v=779610) at jsstr.c:2665 #7 0x205c0 in JS_ValueToString (cx=0xbb998, v=779610) at jsapi.c:543 #8 0x1d4b0 in Print (cx=0xbb998, obj=0xc1420, argc=1, argv=0xcaacc, rval=0xffbfe4c8) at js.c:691 #9 0x4ae24 in js_Invoke (cx=0xbb998, argc=1, flags=0) at jsinterp.c:1354 #10 0x51934 in js_Interpret (cx=0xbb998, pc=0xcaa96 ":", result=0xffbfe6e0) at jsinterp.c:4043 #11 0x4b3b8 in js_Execute (cx=0xbb998, chain=0xffbfe6c0, script=0xcaa60, down=0x0, flags=0, result=0xffbfe804) at jsinterp.c:1613 #12 0x24c70 in JS_ExecuteScript (cx=0xbb998, obj=0xc1420, script=0xcaa60, rval=0xffbfe804) at jsapi.c:4213 #13 0x1c8f4 in Process (cx=0xbb998, obj=0xc1420, filename=0xcaa60 "", forceTTY=4) at js.c:269 #14 0x1d040 in ProcessArgs (cx=0xbb998, obj=0xc1420, argv=0xffbff978, argc=0) at js.c:494 #15 0x1f748 in main (argc=0, argv=0xffbff978, envp=0xffbff97c) at js.c:3158 and 4886718336 = 0x123456780 which means it is printing the answer which is the problem(!) At least it is a clue..
I now made a new distribution from yesterday's CVS-head http://www.newn.cam.ac.uk/prlw/jsref-1.80.tar.gz (Incidentally, bug 366355 didn't quite remove all vestiges of perlconnect, so the job is completed in this tar file) The printing problem (NB output, not actual computation) still exists on sparc-sun-solaris2.9, but trying to build with Makefile.ref failed completely for me anyway, so I don't think this is a regression, and having js/src autoconf'd means it is much easier to test: % gunzip jsref-1.80.tar.gz % tar xvf jsref-1.80 % cd jsref-180 % ./configure --disable-shared % make % gdb js ... (gdb) run ... js> print(0x123456 * 0x10) 19088736 js> print(0x1234567 * 0x10) 305419888 js> print(0x12345678 * 0x10) ^C (hangs on sparc-sun-solaris2.9, not i386-unknown-netbsdelf4.99.20) Program received signal SIGINT, Interrupt. 0x34a94 in mult (a=0xd4324, b=0xd669c) at jsdtoa.c:688 688 z = *x++ * (ULLong)y + *xc + carry; (gdb) bt #0 0x34a94 in mult (a=0xd4324, b=0xd669c) at jsdtoa.c:688 #1 0x34c04 in pow5mult (b=0xd8330, k=21588) at jsdtoa.c:834 #2 0x370fc in js_dtoa (d=4886718336, mode=0, biasUp=0, ndigits=0, decpt=0xffbfe234, sign=0xffbfe230, rve=0xffbfe22c, buf=0xffbfe2b2 "", bufsize=24) at jsdtoa.c:2518 #3 0x377ac in JS_dtostr (buffer=0xffbfe2b0 "", bufferSize=26, mode=DTOSTR_STANDARD, precision=0, d=4886718336) at jsdtoa.c:2796 #4 0x580b0 in js_NumberToString (cx=0xbc940, d=4886718336) at jsnum.c:717 #5 0x8895c in js_ValueToString (cx=0xbc940, v=783714) at jsstr.c:2665 #6 0x207b8 in JS_ValueToString (cx=0xbc940, v=783714) at jsapi.c:543 #7 0x1d6a8 in Print (cx=0xbc940, obj=0xc2420, argc=1, argv=0xcba74, rval=0xffbfe4c8) at js.c:700 #8 0x4afe0 in js_Invoke (cx=0xbc940, argc=1, flags=0) at jsinterp.c:1333 #9 0x51b14 in js_Interpret (cx=0xbc940, pc=0xcba3e ":", result=0xffbfe6e0) at jsinterp.c:4026 #10 0x4b584 in js_Execute (cx=0xbc940, chain=0xffbfe6c0, script=0xcba08, down=0x0, flags=0, result=0xffbfe804) at jsinterp.c:1592 #11 0x24e90 in JS_ExecuteScript (cx=0xbc940, obj=0xc2420, script=0xcba08, rval=0xffbfe804) at jsapi.c:4694 #12 0x1ca64 in Process (cx=0xbc940, obj=0xc2420, filename=0xcba08 "", forceTTY=5) at js.c:265 #13 0x1d220 in ProcessArgs (cx=0xbc940, obj=0xc2420, argv=0xffbff978, argc=0) at js.c:515 #14 0x1f940 in main (argc=0, argv=0xffbff978, envp=0xffbff97c) at js.c:3261 Any chance of testing the distribution / folding in the changes?
Patrick: can you provide the additional perlconnect removal changes as a patch to bug 366355, instead of including them here? We need to tackle a bite-sized chunk in each bug or mayhem ensues. :) Thanks
This is a great bug that should get more love (Patrick, I haven't heard from you in a while, are you still interested in it?), but not a blocker.
Flags: blocking1.9? → blocking1.9-
Not even wanted-1.9?
We now use stdint.h. \o/
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.