301984 - PLHashNumber vs. pointers

Status: NEW

(NSPR :: NSPR, defect)

Description

[Mikhail T.]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.4; FreeBSD; X11; amd64) KHTML/3.4.1 (like Gecko)
Build Identifier: 

In a number of places, PLHashNumber is assumed to be as wide as a pointer. It 
is, however, exactly 32-bit wide, breaking the assumption on some (many?) 
64-bit machines, such as FreeBSD/amd64. 
 
This is the simplest fix: 
 
--- ../lib/ds/plhash.h  Sun Apr 25 11:00:35 2004 
+++ ../lib/ds/plhash.h  Sun Jul 24 23:50:54 2005 
@@ -48,5 +48,5 @@ 
 typedef struct PLHashEntry  PLHashEntry; 
 typedef struct PLHashTable  PLHashTable; 
-typedef PRUint32 PLHashNumber; 
+typedef uintptr_t PLHashNumber; 
 #define PL_HASH_BITS 32  /* Number of bits in PLHashNumber */ 
 typedef PLHashNumber (PR_CALLBACK *PLHashFunction)(const void *key); 
 

Reproducible: Always

Comment 1

[Wan-Teh Chang]

Thanks for the bug report.  We should fix the compiler
warnings by figuring out the right way to compute a
32-bit hash number from a 64-bit pointer, if a typecast
(which results in truncation) is not appropriate.

Please update the bug's summary if you can, or I can do
that after I look at the specific compiler warnings.

Comment 2

[Mikhail T.]

Casting a pointer to a 32-bit integer without -Wall warnings is not difficult: 
 
     uint32_t i = (uintptr_t)p; 
 
But is this desired? May be, on 64-bit machines the hash value ought to be 
64-bit too? 

Comment 3

[Wan-Teh Chang]

No. A hash number is a fixed length number that
"uniquely" identifies an arbitrary length piece
of data.  For PLHashTable, the hash number's
size doesn't need to depend on the size of the
pointer.

Whether casting a 64-bit pointer to a 32-bit
number is desired depends on whether there is
"information" in the upper 32 bits that the
typecast throws away.  In general the hash function
should use all the bits in the input.

Comment 4

[Mikhail T.]

=For PLHashTable, the hash number's size doesn't need to depend 
=on the size of the pointer. 
 
True. But 64-bit machines offer a lot more virtual memory and thus the number 
of objects in hash table(s) increases. Even if these numbers are not sparse, 
the maximum number of objects per table is currently 2^32. This is well above 
the current practical limits, but these limits are growing every year. 
 
If the hash-ing algorithm can handle larger keys, perhaps, it should?.. 

Comment 5

[Wayne Mery (:wsmwk)]

Mikhail T, is this still an issue?

Comment 6

[Mikhail T.]

Well, of course, it is -- and is trivially checked by anyone with access to the most recent nspr/plhash.h

The type PLHashNumber is still typedef-ed as PRUint32.

Comment 7

[Mikhail T.]

Seems like one can not simply clear the needinfo-flag without typing something. My earlier Comment #6 provided the actual answer 3 months ago.

Comment 8

[Wan-Teh Chang]

Hi Mikhail: Thanks. I cleared the needinfo-flag for you.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.