From Bugzilla Helper: User-Agent: Mozilla/4.7 [en] (X11; U; SunOS 5.8 sun4u) BuildID: 20010221 When building mozilla with the Sun Forte 6.0 Update 2 (prerelease) compiler for 64-bit architecture, you get get casting errors in dist/include/nsHashtable.h. However, if you use an identical command line with the 64-bit enabling options removed, it compiles fine and outputs a 32-bit output file. Reproducible: Always Steps to Reproduce: 1. configure with the following command line: CXX=CC CXXFLAGS="-xarch=v9a" CFLAGS="-xarch=v9a" CC=cc ./configure --prefix=/opt/pkgs/mozilla-0.8 --with-x --disable-mailnews --with-jpeg --with-zlib --with-png --disable-debug --enable-ultrasparc --enable-optimize="-fast -xarch=v9a" --disable-tests --enable-strib-libs --enable-nspr-autoconf 2. run make 3. manually compile those few times where CFLAGS is ignored. 4. continue make until the following compilation line: CC -library=iostream -o nsErrorService.o -c -DOSTYPE=\"SunOS5\" -DOJI -D_IMPL_NS_COM -I../../dist/include -I../../dist/include -I/usr/openwin/include -KPIC -xarch=v9a -mt -fast -xarch=v9a -DNDEBUG -DTRIMMED -DMOZILLA_CLIENT -DBROKEN_QSORT=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DMOZ_WIDGET_GTK=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_DEFAULT_TOOLKIT=\"gtk\" -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_64BIT_OS=1 -DHAVE_DIRENT_H=1 -DHAVE_SYS_BYTEORDER_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_SYS_FILIO_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_SYS_VFS_H=1 -DHAVE_SYS_MOUNT_H=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBSOCKET=1 -DHAVE_LIBPOSIX4=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_QSORT=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STATVFS=1 -DHAVE_MEMMOVE=1 -DHAVE_USLEEP=1 -DHAVE_RINT=1 -DHAVE_GETTIMEOFDAY=1 -DGETTIMEOFDAY_TWO_ARGS=1 -DHAVE_DEV_ZERO=1 -DHAVE_IOS_BINARY=1 -DHAVE_OSTREAM=1 -DHAVE_CPP_EXPLICIT=1 -DHAVE_CPP_SPECIALIZATION=1 -DHAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX=1 -DHAVE_CPP_PARTIAL_SPECIALIZATION=1 -DHAVE_CPP_ACCESS_CHANGING_USING=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_NAMESPACE_STD=1 -DHAVE_CPP_UNAMBIGUOUS_STD_NOTEQUAL=1 -DHAVE_CPP_NEW_CASTS=1 -DHAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR=1 -DNEED_CPP_UNUSED_IMPLEMENTATIONS=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_ENDER_LITE=1 -DNS_MT_SUPPORTED=1 -DCPP_CV_QUALIFIERS_CAUSE_AMBIGUITY=1 -DULTRA_SPARC=1 -DMOZ_USER_DIR=\".mozilla\" -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DHAVE_MOVEMAIL=1 -DJS_THREADSAFE=1 nsErrorService.cpp Actual Results: An error from the compiler is returned: "../../dist/include/nsHashtable.h", line 190: Error: Cannot cast from nsISupports*const to unsigned. "../../dist/include/nsHashtable.h", line 219: Error: Cannot cast from void*const to unsigned. 2 Error(s) detected. Expected Results: It should have successfully compiled the code. The code does compile in 32-bit mode though. This strongly suggests a 64-bit cleanliness error.
Marking NEW after talking to Asa.
Bouncing to xpcom.
The casting problem has been solved by bug 20860 but I still think that nsHashTable is not 64-bit clean. For the HashCode of nsISupportsKey & nsVoidKey, it casts the void * of the key object to 32 bits which I think is causing problems in the nsObserverService code.
Looking at http://lxr.mozilla.org/seamonkey/source/nsprpub/lib/ds/plhash.h#46 ... What about getting the following _quick_ workaround implemented: -- snip -- #ifdef USE_64BITS typedef PRUint64 PLHashNumber; #define PL_HASH_BITS 64 /* Number of bits in PLHashNumber */ #else typedef PRUint32 PLHashNumber; #define PL_HASH_BITS 32 /* Number of bits in PLHashNumber */ #endif -- snip -- I know that this is silly, but it provides a clean workaround without introducing other side-effects (except that hash tables take twice the memory) ...
Stealing from scc ...
there may be un-obvious ramifications from a change like this, but it's certainly with trying as an experiment. You'll want to look at places where hashes are generated to see how many obey this macro.
Created attachment 58086 [details] [diff] [review] Patch for mozilla/nsprpub/lib/ds/plhash.h - changing |PLHashNumber| to |PRUint64| on 64bit platforms
Comment on attachment 58086 [details] [diff] [review] Patch for mozilla/nsprpub/lib/ds/plhash.h - changing |PLHashNumber| to |PRUint64| on 64bit platforms Unfortunately it does not fix assertions like: -- snip -- ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.addObserver]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://communicator/content/utilityOverlay.js :: utilityOnLoad :: line 406" data: no] ************************************************************ -- snip -- ;-( Any ideas ?
Does that PLHashNumber change fix anything? It would surprise me if it does, since that should only controls the number of bits in the result of hash functions. seawood - In what way do you think nsISupportsKey and nsVoidKey aren't 64-bit clean? Using the low 32 bits for a hash code should be perfectly clean. Having the same hash code doesn't guaranteed that the values are the same -- it only strongly suggests it, yielding a performance optimization (that would still be just as good even if it's only based on the low 32 bits of the pointer).
A fix for the assertion shown in comment 8 is described in Bug 178499 - boolean values are not being passed correctly in 64-bit mode.
You'll want to ensure that the multiplicative hash uses a 64-bit multiply, to take advantage of this wider size. But as dbaron points out, there is likely to be no gain in hash function effectiveness for commonly-hashed pointer types. /be