PLHashNumber vs. pointers

UNCONFIRMED
Assigned to

Status

UNCONFIRMED
14 years ago
3 years ago

People

(Reporter: mi+mozilla, Assigned: wtc)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

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

Comment 1

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

Comment 2

14 years ago
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? 
Summary: PLHashNumber should be as wide as a pointer → PLHashNumber vs. pointers
(Assignee)

Comment 3

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

Comment 4

14 years ago
=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?.. 
QA Contact: wtchang → nspr

Comment 5

3 years ago
Mikhail T, is this still an issue?
Flags: needinfo?(mi+mozilla)
(Reporter)

Comment 6

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

Comment 7

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

Comment 8

3 years ago
Hi Mikhail: Thanks. I cleared the needinfo-flag for you.
Flags: needinfo?(mi+mozilla)
You need to log in before you can comment on or make changes to this bug.