ICE-4702 - Poor hash algorithm

10 files changed, 47 insertions, 196 deletions

diff --git a/cpp/src/Ice/HashUtil.h b/cpp/src/Ice/HashUtil.h
index 7b6c7f6a27b..d23ee90ff0c 100644
--- a/cpp/src/Ice/HashUtil.h
+++ b/cpp/src/Ice/HashUtil.h
@@ -15,13 +15,13 @@ namespace IceInternal
 inline void
 hashAdd(Ice::Int& hashCode, Ice::Int value)
 {
-    hashCode = hashCode * 5 + value;
+    hashCode = ((hashCode << 5) + hashCode) ^ (2654435761u * value);
 }
 
 inline void
 hashAdd(Ice::Int& hashCode, bool value)
 {
-    hashCode = hashCode * 5 + static_cast<Ice::Int>(value);
+    hashCode = ((hashCode << 5) + hashCode) ^ (value ? 1 : 0);
 }
 
 inline void
@@ -29,7 +29,7 @@ hashAdd(Ice::Int& hashCode, const std::string& value)
 {
     for(std::string::const_iterator p = value.begin(); p != value.end(); ++p)
     {
-        hashCode = 5 * hashCode + *p;
+        hashCode = ((hashCode << 5) + hashCode) ^ *p;
     }    
 }
 
@@ -52,5 +52,4 @@ hashAdd(Ice::Int& hashCode, const std::map<K, V>& map)
     }
 }
 
-
 }
diff --git a/cpp/src/Ice/Makefile.mak b/cpp/src/Ice/Makefile.mak
index 522da39075e..b5378cba7ef 100644
--- a/cpp/src/Ice/Makefile.mak
+++ b/cpp/src/Ice/Makefile.mak
@@ -124,7 +124,6 @@ CPPFLAGS	= $(CPPFLAGS) -DCOMPSUFFIX=\"$(COMPSUFFIX)\"
 SLICE2CPPFLAGS	= --ice --include-dir Ice --dll-export ICE_API $(SLICE2CPPFLAGS)

 LINKWITH        = $(BASELIBS) $(BZIP2_LIBS) $(ICE_OS_LIBS) ws2_32.lib Iphlpapi.lib

 

-

 PDBFLAGS        = /pdb:$(DLLNAME:.dll=.pdb)

 LD_DLLFLAGS	= $(LD_DLLFLAGS) /entry:"ice_DLL_Main"

 RES_FILE	= Ice.res

diff --git a/cpp/src/Ice/OpaqueEndpointI.cpp b/cpp/src/Ice/OpaqueEndpointI.cpp
index a3ed87efd85..80e3146a327 100644
--- a/cpp/src/Ice/OpaqueEndpointI.cpp
+++ b/cpp/src/Ice/OpaqueEndpointI.cpp
@@ -433,7 +433,8 @@ IceInternal::OpaqueEndpointI::operator<(const LocalObject& r) const
 Ice::Int
 IceInternal::OpaqueEndpointI::hashInit() const
 {
-    Ice::Int h = _type;
+    Ice::Int h = 5381;
+    hashAdd(h, _type);
     hashAdd(h, _rawEncoding.major);
     hashAdd(h, _rawEncoding.minor);
     hashAdd(h, _rawBytes);
diff --git a/cpp/src/Ice/Reference.cpp b/cpp/src/Ice/Reference.cpp
index 3ec8385c214..0b8a6eaff7b 100644
--- a/cpp/src/Ice/Reference.cpp
+++ b/cpp/src/Ice/Reference.cpp
@@ -483,12 +483,20 @@ IceInternal::Reference::Reference(const Reference& r) :
 int
 IceInternal::Reference::hashInit() const
 {
-    Int h = static_cast<Int>(_mode);
+    Int h = 5381;
+    hashAdd(h, static_cast<Int>(_mode));
+    hashAdd(h, _secure);
     hashAdd(h, _identity.name);
     hashAdd(h, _identity.category);
     hashAdd(h, _context->getValue());
     hashAdd(h, _facet);
-    hashAdd(h, _secure);
+    hashAdd(h, _overrideCompress);
+    if(_overrideCompress)
+    {
+        hashAdd(h, _compress);
+    }
+    hashAdd(h, _encoding.major);
+    hashAdd(h, _encoding.minor);
     return h;
 }
 
diff --git a/cpp/src/Ice/TcpEndpointI.cpp b/cpp/src/Ice/TcpEndpointI.cpp
index e38885976fb..9c64b22057b 100644
--- a/cpp/src/Ice/TcpEndpointI.cpp
+++ b/cpp/src/Ice/TcpEndpointI.cpp
@@ -547,10 +547,15 @@ IceInternal::TcpEndpointI::operator<(const LocalObject& r) const
 Ice::Int
 IceInternal::TcpEndpointI::hashInit() const
 {
-    Ice::Int h = 0;
+    Ice::Int h = 5381;
+    hashAdd(h, TCPEndpointType);
     hashAdd(h, _host);
     hashAdd(h, _port);
     hashAdd(h, _timeout);
+    hashAdd(h, _protocol.major);
+    hashAdd(h, _protocol.minor);
+    hashAdd(h, _encoding.major);
+    hashAdd(h, _encoding.minor);
     hashAdd(h, _connectionId);
     hashAdd(h, _compress);
     return h;
diff --git a/cpp/src/Ice/UdpEndpointI.cpp b/cpp/src/Ice/UdpEndpointI.cpp
index f8dacf01d7a..db5f1a495b7 100644
--- a/cpp/src/Ice/UdpEndpointI.cpp
+++ b/cpp/src/Ice/UdpEndpointI.cpp
@@ -632,12 +632,17 @@ IceInternal::UdpEndpointI::operator<(const LocalObject& r) const
 Ice::Int
 IceInternal::UdpEndpointI::hashInit() const
 {
-    Ice::Int h = 0;
+    Ice::Int h = 5381;
+    hashAdd(h, UDPEndpointType);
     hashAdd(h, _host);
     hashAdd(h, _port);
     hashAdd(h, _mcastInterface);
     hashAdd(h, _mcastTtl);
     hashAdd(h, _connect);
+    hashAdd(h, _protocol.major);
+    hashAdd(h, _protocol.minor);
+    hashAdd(h, _encoding.major);
+    hashAdd(h, _encoding.minor);
     hashAdd(h, _connectionId);
     hashAdd(h, _compress);
     return h;
diff --git a/cpp/src/Ice/winrt/StreamEndpointI.cpp b/cpp/src/Ice/winrt/StreamEndpointI.cpp
index 1f1c030f84e..d048945f0a0 100644
--- a/cpp/src/Ice/winrt/StreamEndpointI.cpp
+++ b/cpp/src/Ice/winrt/StreamEndpointI.cpp
@@ -587,10 +587,15 @@ IceInternal::StreamEndpointI::operator<(const LocalObject& r) const
 Ice::Int
 IceInternal::StreamEndpointI::hashInit() const
 {
-    Ice::Int h = 0;
+    Ice::Int h = 5381;
+    hashAdd(h, _type);
     hashAdd(h, _host);
     hashAdd(h, _port);
     hashAdd(h, _timeout);
+    hashAdd(h, _protocol.major);
+    hashAdd(h, _protocol.minor);
+    hashAdd(h, _encoding.major);
+    hashAdd(h, _encoding.minor);
     hashAdd(h, _connectionId);
     hashAdd(h, _compress);
     return h;
diff --git a/cpp/src/IceSSL/EndpointI.cpp b/cpp/src/IceSSL/EndpointI.cpp
index a9eb8e707d8..0b33e896741 100644
--- a/cpp/src/IceSSL/EndpointI.cpp
+++ b/cpp/src/IceSSL/EndpointI.cpp
@@ -544,10 +544,15 @@ IceSSL::EndpointI::operator<(const Ice::LocalObject& r) const
 Ice::Int
 IceSSL::EndpointI::hashInit() const
 {
-    Int h = 0;
+    Int h = 5381;
+    IceInternal::hashAdd(h, EndpointType);
     IceInternal::hashAdd(h, _host);
     IceInternal::hashAdd(h, _port);
     IceInternal::hashAdd(h, _timeout);
+    IceInternal::hashAdd(h, _protocol.major);
+    IceInternal::hashAdd(h, _protocol.minor);
+    IceInternal::hashAdd(h, _encoding.major);
+    IceInternal::hashAdd(h, _encoding.minor);
     IceInternal::hashAdd(h, _connectionId);
     IceInternal::hashAdd(h, _compress);
     return h;
diff --git a/cpp/src/slice2cs/Gen.cpp b/cpp/src/slice2cs/Gen.cpp
index f636b758a77..f76877a2065 100644
--- a/cpp/src/slice2cs/Gen.cpp
+++ b/cpp/src/slice2cs/Gen.cpp
@@ -2844,8 +2844,9 @@ Slice::Gen::TypesVisitor::visitExceptionEnd(const ExceptionPtr& p)
     }
     else
     {
-        _out << nl << "int h__ = 0;";
+        _out << nl << "int h__ = 5381;";
     }
+    _out << nl << "IceInternal.HashUtil.hashAdd(ref h__, \"" << p->scoped() << "\");";
     writeMemberHashCode(dataMembers, DotNet::Exception);
     _out << nl << "return h__;";
     _out << eb;
@@ -3252,7 +3253,8 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p)
     emitGeneratedCodeAttribute();
     _out << nl << "public override int GetHashCode()";
     _out << sb;
-    _out << nl << "int h__ = 0;";
+    _out << nl << "int h__ = 5381;";
+    _out << nl << "IceInternal.HashUtil.hashAdd(ref h__, \"" << p->scoped() << "\");";
     writeMemberHashCode(dataMembers, isClass ? DotNet::ICloneable : 0);
     _out << nl << "return h__;";
     _out << eb;
@@ -3678,73 +3680,7 @@ Slice::Gen::TypesVisitor::writeMemberHashCode(const DataMemberList& dataMembers,
 {
     for(DataMemberList::const_iterator q = dataMembers.begin(); q != dataMembers.end(); ++q)
     {
-        string memberName = fixId((*q)->name(), baseTypes);
-        TypePtr memberType = (*q)->type();
-        bool isValue = isValueType(memberType);
-        if(!isValue)
-        {
-            _out << nl << "if(" << memberName << " != null)";
-            _out << sb;
-        }
-        SequencePtr seq = SequencePtr::dynamicCast(memberType);
-        if(seq)
-        {
-            string meta;
-            bool isSerializable = seq->findMetaData("clr:serializable", meta);
-            bool isGeneric = seq->findMetaData("clr:generic:", meta);
-            bool isArray = !isSerializable && !isGeneric && !seq->hasMetaData("clr:collection");
-            if(isArray)
-            {
-                //
-                // GetHashCode() for native arrays does not have value semantics.
-                //
-                _out << nl << "h__ = 5 * h__ + IceUtilInternal.Arrays.GetHashCode(" << memberName << ");";
-            }
-            else if(isGeneric)
-            {
-                //
-                // GetHashCode() for generic types does not have value semantics.
-                //
-                _out << nl << "h__ = 5 * h__ + IceUtilInternal.Collections.SequenceGetHashCode(" << memberName << ");";
-            }
-            else
-            {
-                //
-                // GetHashCode() for CollectionBase has value semantics.
-                //
-                _out << nl << "h__ = 5 * h__ + " << memberName << ".GetHashCode();";
-            }
-        }
-        else
-        {
-            DictionaryPtr dict = DictionaryPtr::dynamicCast(memberType);
-            if(dict)
-            {
-                if(dict->hasMetaData("clr:collection"))
-                {
-                    //
-                    // GetHashCode() for DictionaryBase has value semantics.
-                    //
-                    _out << nl << "h__ = 5 * h__ + " << memberName << ".GetHashCode();";
-                }
-                else
-                {
-                    //
-                    // GetHashCode() for generic types does not have value semantics.
-                    //
-                    _out << nl << "h__ = 5 * h__ + IceUtilInternal.Collections.DictionaryGetHashCode(" << memberName
-                         << ");";
-                }
-            }
-            else
-            {
-                _out << nl << "h__ = 5 * h__ + " << memberName << ".GetHashCode();";
-            }
-        }
-        if(!isValue)
-        {
-            _out << eb;
-        }
+        _out << nl << "IceInternal.HashUtil.hashAdd(ref h__, " << fixId((*q)->name(), baseTypes) << ");";
     }
 }
 
diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp
index 3f0c23c2aca..1a9b678b6aa 100644
--- a/cpp/src/slice2java/Gen.cpp
+++ b/cpp/src/slice2java/Gen.cpp
@@ -465,110 +465,6 @@ Slice::JavaVisitor::writeDelegateThrowsClause(const string& package, const Excep
 }
 
 void
-Slice::JavaVisitor::writeHashCode(Output& out, const TypePtr& type, const string& name, int& iter,
-                                  const StringList& metaData)
-{
-    BuiltinPtr builtin = BuiltinPtr::dynamicCast(type);
-    if(builtin)
-    {
-        switch(builtin->kind())
-        {
-            case Builtin::KindByte:
-            case Builtin::KindShort:
-            case Builtin::KindLong:
-            {
-                out << nl << "__h = 5 * __h + (int)" << name << ';';
-                break;
-            }
-            case Builtin::KindBool:
-            {
-                out << nl << "__h = 5 * __h + (" << name << " ? 1 : 0);";
-                break;
-            }
-            case Builtin::KindInt:
-            {
-                out << nl << "__h = 5 * __h + " << name << ';';
-                break;
-            }
-            case Builtin::KindFloat:
-            {
-                out << nl << "__h = 5 * __h + java.lang.Float.floatToIntBits(" << name << ");";
-                break;
-            }
-            case Builtin::KindDouble:
-            {
-                out << nl << "__h = 5 * __h + (int)java.lang.Double.doubleToLongBits(" << name << ");";
-                break;
-            }
-            case Builtin::KindString:
-            {
-                out << nl << "if(" << name << " != null)";
-                out << sb;
-                out << nl << "__h = 5 * __h + " << name << ".hashCode();";
-                out << eb;
-                break;
-            }
-            case Builtin::KindObject:
-            case Builtin::KindObjectProxy:
-            case Builtin::KindLocalObject:
-            {
-                out << nl << "if(" << name << " != null)";
-                out << sb;
-                out << nl << "__h = 5 * __h + " << name << ".hashCode();";
-                out << eb;
-                break;
-            }
-        }
-        return;
-    }
-
-    ProxyPtr prx = ProxyPtr::dynamicCast(type);
-    ClassDeclPtr cl = ClassDeclPtr::dynamicCast(type);
-    DictionaryPtr dict = DictionaryPtr::dynamicCast(type);
-    if(prx || cl || dict)
-    {
-        out << nl << "if(" << name << " != null)";
-        out << sb;
-        out << nl << "__h = 5 * __h + " << name << ".hashCode();";
-        out << eb;
-        return;
-    }
-
-    SequencePtr seq = SequencePtr::dynamicCast(type);
-    if(seq)
-    {
-        bool customType = hasTypeMetaData(seq, metaData);
-
-        out << nl << "if(" << name << " != null)";
-        out << sb;
-        if(customType)
-        {
-            out << nl << "__h = 5 * __h + " << name << ".hashCode();";
-        }
-        else
-        {
-            out << nl << "for(int __i" << iter << " = 0; __i" << iter << " < " << name << ".length; __i" << iter
-                << "++)";
-            out << sb;
-            ostringstream elem;
-            elem << name << "[__i" << iter << ']';
-            iter++;
-            writeHashCode(out, seq->type(), elem.str(), iter);
-            out << eb;
-        }
-        out << eb;
-        return;
-    }
-
-    ConstructedPtr constructed = ConstructedPtr::dynamicCast(type);
-    assert(constructed);
-    out << nl << "if(" << name << " != null)";
-    out << sb;
-    out << nl << "__h = 5 * __h + " << name << ".hashCode();";
-    out << eb;
-}
-
-void
 Slice::JavaVisitor::writeMarshalDataMember(Output& out, const string& package, const DataMemberPtr& member, int& iter)
 {
     if(!member->optional())
@@ -2528,14 +2424,6 @@ Slice::Gen::TieVisitor::visitClassDefStart(const ClassDefPtr& p)
 
     if(p->isLocal())
     {
-        out << sp << nl << "/**";
-        out << nl << " * @deprecated This method is deprecated, use hashCode instead.";
-        out << nl << " **/";
-        out << nl << "public int" << nl << "ice_hash()";
-        out << sb;
-        out << nl << "return hashCode();";
-        out << eb;
-
         out << sp << nl << "public java.lang.Object" << nl << "clone()";
         out.inc();
         out << nl << "throws java.lang.CloneNotSupportedException";
@@ -3772,13 +3660,13 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p)
 
     out << sp << nl << "public int" << nl << "hashCode()";
     out << sb;
-    out << nl << "int __h = 0;";
+    out << nl << "int __h = 5381;";
+    out << nl << "__h = IceInternal.HashUtil.hashAdd(__h, \"" << p->scoped() << "\");";
     iter = 0;
     for(d = members.begin(); d != members.end(); ++d)
     {
         string memberName = fixKwd((*d)->name());
-        StringList metaData = (*d)->getMetaData();
-        writeHashCode(out, (*d)->type(), memberName, iter, metaData);
+        out << nl << "__h = IceInternal.HashUtil.hashAdd(__h, " << memberName << ");";
     }
     out << nl << "return __h;";
     out << eb;