361075 - [FIX] prmjtime.c:368: error: incompatible types in assignment JSLL_MUL JSInt64

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Patrick Welche]

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

Under NetBSD-current/i386, gmake -f client.mk i.e., cvs code from just now:

gcc -o prmjtime.o -c -fvisibility=hidden -DOSTYPE=\"NetBSD4.99\" -DOSARCH=\"NetBSD\" -DBUILD_ID=0000000000 -DEXPORT_JS_API  -DJS_USE_SAFE_ARENA   -I../../dist/include   -I../../dist/include/js -I../../dist/include/nspr  -DMOZ_PNG_READ -DMOZ_PNG_WRITE -I/usr/include -I../../dist/sdk/include -I. -I/usr/X11R6/include   -fPIC -DPIC -I/usr/X11R6/include -Wall -W -Wno-unused -Wpointer-arith -Wcast-align -Wno-long-long -pedantic -Dunix -pthread -pipe  -DDEBUG -D_DEBUG -DDEBUG_prlw1 -DTRACING -g -O  -I/usr/X11R6/include -include ../../mozilla-config.h -DMOZILLA_CLIENT -Wp,-MD,.deps/prmjtime.pp prmjtime.c
prmjtime.c: In function 'PRMJ_basetime':
prmjtime.c:368: error: incompatible types in assignment
prmjtime.c:371: error: incompatible types in assignment
prmjtime.c:374: error: incompatible types in assignment
gmake[4]: *** [prmjtime.o] Error 1
gmake[4]: Leaving directory `/usr/src/local/mozilla/js/src'

The lines in question all involve JSLL_MUL, defined in jslong.h:

#ifdef JS_HAVE_LONG_LONG
#define JSLL_MUL(r, a, b)        ((r) = (a) * (b))
#else  /* !JS_HAVE_LONG_LONG */
#define JSLL_MUL(r, a, b) { \
    JSInt64 _a, _b; \
    _a = a; _b = b; \
    JSLL_MUL32(r, _a.lo, _b.lo); \
    (r).hi += _a.hi * _b.lo + _a.lo * _b.hi; \
}
...
#endif
(Does that want to be JSLL_MUL32(r.lo, _a.lo... ?)

Ah.. jsosdep.h doesn't look very sensible - what ever happened to the autoconf philosophy of "test features, not operating systems"?

Carrying on the rot, I added "#define JS_HAVE_LONG_LONG" to jsosdep.h, and compilation of libmozjs succeeded.

So, we have 2 problems:
1) jsosdep.h is not the right solution - test for long long, and define JS_HAVE_LONG_LONG accordingly.
2) JSLL_MUL is broken in the !JS_HAVE_LONG_LONG case.

Reproducible: Always

Steps to Reproduce:
1.find a NetBSD test box
2.gmake -f client.mk


Actual Results:  
...
gcc -o prmjtime.o -c -fvisibility=hidden -DOSTYPE=\"NetBSD4.99\" -DOSARCH=\"NetBSD\" -DBUILD_ID=0000000000 -DEXPORT_JS_API  -DJS_USE_SAFE_ARENA   -I../../dist/include   -I../../dist/include/js -I../../dist/include/nspr  -DMOZ_PNG_READ -DMOZ_PNG_WRITE -I/usr/include -I../../dist/sdk/include -I. -I/usr/X11R6/include   -fPIC -DPIC -I/usr/X11R6/include -Wall -W -Wno-unused -Wpointer-arith -Wcast-align -Wno-long-long -pedantic -Dunix -pthread -pipe  -DDEBUG -D_DEBUG -DDEBUG_prlw1 -DTRACING -g -O  -I/usr/X11R6/include -include ../../mozilla-config.h -DMOZILLA_CLIENT -Wp,-MD,.deps/prmjtime.pp prmjtime.c
prmjtime.c: In function 'PRMJ_basetime':
prmjtime.c:368: error: incompatible types in assignment
prmjtime.c:371: error: incompatible types in assignment
prmjtime.c:374: error: incompatible types in assignment
gmake[4]: *** [prmjtime.o] Error 1
gmake[4]: Leaving directory `/usr/src/local/mozilla/js/src'
gmake[3]: *** [libs_tier_js] Error 2
gmake[3]: Leaving directory `/usr/src/local/mozilla'
gmake[2]: *** [tier_js] Error 2
gmake[2]: Leaving directory `/usr/src/local/mozilla'
gmake[1]: *** [alldep] Error 2
gmake[1]: Leaving directory `/usr/src/local/mozilla'
gmake: *** [alldep] Error 2


Expected Results:  
chmod +x libmozjs.so
/usr/src/local/mozilla/config/nsinstall -R -m 755 libmozjs.so ../../dist/lib
/usr/src/local/mozilla/config/nsinstall -R -m 755 libmozjs.so ../../dist/bin
/usr/src/local/mozilla/config/nsinstall -R -m 755 host_jskwgen ../../dist/host/bin


1) (not tested, and for autoconf > 2.13) AC_CHECK_TYPES([long long],[AC_DEFINE([JS_HAVE_LONG_LONG]))
2) fix JSLL_MUL - should that call to JSLL_MUL32 be with r.lo ?

Comment 1

[Boris Zbarsky [:bzbarsky]]

> (Does that want to be JSLL_MUL32(r.lo, _a.lo... ?)

No, since the product of two 32-bit ints could overflow a 32-bit int.  JSLL_MUL32 splits up things into 16-bit ints, and sets both r.lo and r.hi in general.

The real problem is that the patch for bug 360484 passes a |long| in as the third arg to JSLL_MUL.  That's not gonna work.

Let's fix that in this bug, and get a separate bug filed on the configure change (which is outside my area of expertise)?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Patrick, does this fix things for you?

Comment 3

[Patrick Welche]

Yes, your patch fixes the compile problem, thanks!

Will you open a new bug about removing jsosdep.h?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I could if you'd really rather not.  If you're willing to, though, please go for it.  cc brendan@moz, shaver@moz, me, and mrbkap?

Comment 5

[Patrick Welche]

LONG_LONG issues now in bug 361075. (BTW I don't now how to cc: mrbkap)

Comment 6

[Boris Zbarsky [:bzbarsky]]

This isn't fixed -- the patch hasn't been checked in yet.

> (BTW I don't now how to cc: mrbkap)

You type "mrbkap" in the CC field.  ;)

Comment 7

[Patrick Welche]

Erratum: Comment 5 should read: LONG_LONG issues now in bug 361268.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 245862 [details] [diff] [review]
Fix

Every time I look at JSLL_-using code, I feel like I've lost a bet with the universe. r=shaver

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yeah, the JSLL stuff is a trifle nasty.  ;)  Checked in.

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed (checked in) mozilla/js/src/prmjtime.c 3.57