erts: Discontinue salted use of enif_hash/phash2

7 files changed, 32 insertions, 33 deletions

diff --git a/erts/doc/src/erl_nif.xml b/erts/doc/src/erl_nif.xml
index 534c90cbdc..05b519fe7d 100644
--- a/erts/doc/src/erl_nif.xml
+++ b/erts/doc/src/erl_nif.xml
@@ -829,10 +829,10 @@ typedef enum {
           <item>
             <p>Portable hash function that gives the same hash for the
               same Erlang term regardless of machine architecture and ERTS version.</p>
-            <p>It takes 32-bit salt values and generates hashes within <c>0..2^27-1</c>.</p>
+            <p><em>It ignores salt values</em> and generates hashes within <c>0..2^27-1</c>.</p>
             <p>Slower than <c>ERL_NIF_INTERNAL_HASH.</c>
-              When used with <c>salt=0</c>, it corresponds to
-              <seealso marker="erlang#phash2-1"><c>erlang:phash2/1</c></seealso>.</p>
+              It corresponds to <seealso marker="erlang#phash2-1"><c>erlang:phash2/1</c></seealso>.
+            </p>
           </item>
         </taglist>
       </item>
@@ -1418,7 +1418,7 @@ typedef enum {
       <desc>
         <p>Hashes <c>term</c> according to the specified
           <seealso marker="#ErlNifHash"><c>ErlNifHash</c></seealso> <c>type</c>.</p>
-        <p>Ranges of taken salt and returned value depend on the hash type.</p>
+        <p>Ranges of taken salt (if any) and returned value depend on the hash type.</p>
       </desc>
     </func>
 
diff --git a/erts/emulator/beam/bif.c b/erts/emulator/beam/bif.c
index 7473a8e43f..214de3652f 100644
--- a/erts/emulator/beam/bif.c
+++ b/erts/emulator/beam/bif.c
@@ -4881,7 +4881,7 @@ BIF_RETTYPE phash2_1(BIF_ALIST_1)
 {
     Uint32 hash;
 
-    hash = make_hash2(BIF_ARG_1, 0);
+    hash = make_hash2(BIF_ARG_1);
     BIF_RET(make_small(hash & ((1L << 27) - 1)));
 }
 
@@ -4901,7 +4901,7 @@ BIF_RETTYPE phash2_2(BIF_ALIST_2)
 	}
 	range = (Uint32) u;
     }
-    hash = make_hash2(BIF_ARG_1, 0);
+    hash = make_hash2(BIF_ARG_1);
     if (range) {
 	final_hash = hash % range; /* [0..range-1] */
     } else {
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index 3ff6c8d3ed..e101747011 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -1220,7 +1220,11 @@ ErlNifUInt64 enif_hash(ErlNifHash type, Eterm term, ErlNifUInt64 salt)
         case ERL_NIF_INTERNAL_HASH:
             return make_internal_hash(term, (Uint32) salt);
         case ERL_NIF_PHASH2:
-            return make_hash2(term, (Uint32) salt) & ((1 << 27) - 1);
+            /* It appears that make_hash2 doesn't always react to seasoning
+             * as well as it should. Therefore, let's make it ignore the salt
+             * value and declare salted uses of phash2 as unsupported.
+             */
+            return make_hash2(term) & ((1 << 27) - 1);
         default:
             return 0;
     }
diff --git a/erts/emulator/beam/erl_utils.h b/erts/emulator/beam/erl_utils.h
index b38beaf93d..75d7e47239 100644
--- a/erts/emulator/beam/erl_utils.h
+++ b/erts/emulator/beam/erl_utils.h
@@ -118,7 +118,7 @@ int erts_fit_in_bits_uint(Uint);
 Sint erts_list_length(Eterm);
 int erts_is_builtin(Eterm, Eterm, int);
 Uint32 block_hash(byte *, unsigned, Uint32);
-Uint32 make_hash2(Eterm, Uint32 salt);
+Uint32 make_hash2(Eterm);
 Uint32 make_hash(Eterm);
 Uint32 make_internal_hash(Eterm, Uint32 salt);
 
diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c
index 2378c057dd..9d140470ac 100644
--- a/erts/emulator/beam/utils.c
+++ b/erts/emulator/beam/utils.c
@@ -1003,7 +1003,7 @@ tail_recur:
 	    break;
 	}
     case MAP_DEF:
-        hash = hash*FUNNY_NUMBER13 + FUNNY_NUMBER14 + make_hash2(term, 0);
+        hash = hash*FUNNY_NUMBER13 + FUNNY_NUMBER14 + make_hash2(term);
         break;
     case TUPLE_DEF:
 	{
@@ -1109,7 +1109,7 @@ block_hash(byte *k, unsigned length, Uint32 initval)
 }
 
 Uint32
-make_hash2(Eterm term, Uint32 salt)
+make_hash2(Eterm term)
 {
     Uint32 hash;
     Uint32 hash_xor_pairs;
@@ -1179,7 +1179,7 @@ make_hash2(Eterm term, Uint32 salt)
 	    switch (term & _TAG_IMMED2_MASK) {
 	    case _TAG_IMMED2_ATOM:
 		/* Fast, but the poor hash value should be mixed. */
-		return atom_tab(atom_val(term))->slot.bucket.hvalue ^ salt;
+		return atom_tab(atom_val(term))->slot.bucket.hvalue;
 	    }
 	    break;
 	case _TAG_IMMED1_SMALL:
@@ -1190,7 +1190,7 @@ make_hash2(Eterm term, Uint32 salt)
 		  term = small_to_big(x, tmp_big);
 		  break;
 	      }
-	      hash = salt;
+	      hash = 0;
 	      SINT32_HASH(x, HCONST);
 	      return hash;
 	  }
@@ -1201,7 +1201,7 @@ make_hash2(Eterm term, Uint32 salt)
     DECLARE_ESTACK(s);
 
     UseTmpHeapNoproc(2);
-    hash = salt;
+    hash = 0;
     for (;;) {
 	switch (primary_tag(term)) {
 	case TAG_PRIMARY_LIST:
@@ -1277,7 +1277,7 @@ make_hash2(Eterm term, Uint32 salt)
                     ESTACK_PUSH(s, hash_xor_pairs);
                     ESTACK_PUSH(s, hash);
                     ESTACK_PUSH(s, HASH_MAP_TAIL);
-                    hash = salt;
+                    hash = 0;
                     hash_xor_pairs = 0;
                     for (i = size - 1; i >= 0; i--) {
                         ESTACK_PUSH(s, HASH_MAP_PAIR);
@@ -1296,7 +1296,7 @@ make_hash2(Eterm term, Uint32 salt)
                     ESTACK_PUSH(s, hash_xor_pairs);
                     ESTACK_PUSH(s, hash);
                     ESTACK_PUSH(s, HASH_MAP_TAIL);
-                    hash = salt;
+                    hash = 0;
                     hash_xor_pairs = 0;
                 }
                 switch (hdr & _HEADER_MAP_SUBTAG_MASK) {
@@ -1471,7 +1471,7 @@ make_hash2(Eterm term, Uint32 salt)
 	    break;
 
 	    default:
-		erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X, %lu)\n", term, salt);
+		erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X)\n", term);
 	    }
 	}
 	break;
@@ -1488,21 +1488,21 @@ make_hash2(Eterm term, Uint32 salt)
 	    case _TAG_IMMED1_IMMED2:
 		switch (term & _TAG_IMMED2_MASK) {
 		case _TAG_IMMED2_ATOM:
-		    if (hash == salt)
+		    if (hash == 0)
 			/* Fast, but the poor hash value should be mixed. */
-			hash = atom_tab(atom_val(term))->slot.bucket.hvalue ^ salt;
+			hash = atom_tab(atom_val(term))->slot.bucket.hvalue;
 		    else
 			UINT32_HASH(atom_tab(atom_val(term))->slot.bucket.hvalue,
 				    HCONST_3);
 		    goto hash2_common;
 		case _TAG_IMMED2_NIL:
-		    if (hash == salt)
-			hash = 3468870702UL ^ salt;
+		    if (hash == 0)
+			hash = 3468870702UL;
 		    else
 			UINT32_HASH(NIL_DEF, HCONST_2);
 		    goto hash2_common;
 		default:
-		    erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X, %lu)\n", term, salt);
+		    erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X)\n", term);
 		}
 	    case _TAG_IMMED1_SMALL:
 	      {
@@ -1518,7 +1518,7 @@ make_hash2(Eterm term, Uint32 salt)
 	    }
 	    break;
 	default:
-	    erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X, %lu)\n", term, salt);
+	    erts_exit(ERTS_ERROR_EXIT, "Invalid tag in make_hash2(0x%X)\n", term);
 	hash2_common:
 
 	    /* Uint32 hash always has the hash value of the previous term,
@@ -1542,7 +1542,7 @@ make_hash2(Eterm term, Uint32 salt)
 		}
 		case HASH_MAP_PAIR:
 		    hash_xor_pairs ^= hash;
-                    hash = salt;
+                    hash = 0;
 		    goto hash2_common;
 		default:
 		    break;
diff --git a/erts/emulator/hipe/hipe_bif0.c b/erts/emulator/hipe/hipe_bif0.c
index b7297043e5..8b420b9e9b 100644
--- a/erts/emulator/hipe/hipe_bif0.c
+++ b/erts/emulator/hipe/hipe_bif0.c
@@ -496,7 +496,7 @@ static ErlOffHeap const_term_table_off_heap;
 
 static HashValue const_term_hash(void *tmpl)
 {
-    return make_hash2((Eterm)tmpl, 0);
+    return make_hash2((Eterm)tmpl);
 }
 
 static int const_term_cmp(void *tmpl, void *bucket)
diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index 6e4866bfd8..8ad11d3bf3 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -59,8 +59,7 @@
          nif_snprintf/1,
          nif_internal_hash/1,
          nif_internal_hash_salted/1,
-         nif_phash2/1,
-         nif_phash2_salted/1
+         nif_phash2/1
 	]).
 
 -export([many_args_100/100]).
@@ -97,8 +96,7 @@ all() ->
      nif_snprintf,
      nif_internal_hash,
      nif_internal_hash_salted,
-     nif_phash2,
-     nif_phash2_salted].
+     nif_phash2].
 
 groups() ->
     [{G, [], api_repeaters()} || G <- api_groups()]
@@ -2637,7 +2635,8 @@ nif_phash2(Config) ->
         lists:map(
           fun (Term) ->
                   HashValue = erlang:phash2(Term),
-                  NifHashValue = hash_nif(phash2, Term, 0),
+                  Salt = random_uint32(), % phash2 should ignore salt
+                  NifHashValue = hash_nif(phash2, Term, Salt),
                   (HashValue =:= NifHashValue
                    orelse ct:fail("Expected: ~p\nActual:   ~p",
                                   [HashValue, NifHashValue])),
@@ -2646,10 +2645,6 @@ nif_phash2(Config) ->
           Terms),
     test_bit_distribution_fitness(HashValues, HashValueBitSize, 0.05).
 
-nif_phash2_salted(Config) ->
-    ensure_lib_loaded(Config),
-    test_salted_nif_hash(phash2).
-
 test_salted_nif_hash(HashType) ->
     HashValueBitSize = nif_hash_result_bitsize(HashType),
     Terms = unique([random_term() || _ <- lists:seq(1, 5000)]),