From 2a0ff8b50a68d51e44fddf36867f390b64a4f2bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn-Egil=20Dahlberg?= <egil@erlang.org>
Date: Wed, 1 Apr 2015 15:30:12 +0200
Subject: erts: Ensure hashing of zero is consistent

Erlang treats positive and negative zero as equal, meaning,

    true = 0.0 =:= 0.0/-1

However, Erlangs hash functions: hash, phash and phash2 did not
reflect this behaviour. Meaning, the hash values produced by
the different hash functions would not be identical for positive
and negative zero.

This commit ensures that hash value of positive zero is always
produced regardless of the signedness of the zero float, i.e.

    true = erlang:phash2(0.0) =:= erlang:phash2(0.0/-1)
---
 erts/emulator/beam/utils.c        | 29 ++++++++++++++++++-----------
 erts/emulator/test/hash_SUITE.erl | 27 ++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c
index 2dd81e3ca3..2018518bf3 100644
--- a/erts/emulator/beam/utils.c
+++ b/erts/emulator/beam/utils.c
@@ -955,12 +955,14 @@ tail_recur:
 	UINT32_HASH_RET(external_ref_numbers(term)[0],FUNNY_NUMBER9,FUNNY_NUMBER10);
     case FLOAT_DEF: 
 	{
-	    FloatDef ff;
-	    GET_DOUBLE(term, ff);
-	    hash = hash*FUNNY_NUMBER6 + (ff.fw[0] ^ ff.fw[1]);
-	    break;
+            FloatDef ff;
+            GET_DOUBLE(term, ff);
+            if (ff.fd == 0.0f) {
+                ff.fd = 0.0f; /* ensure pos. 0.0 */
+            }
+            hash = hash*FUNNY_NUMBER6 + (ff.fw[0] ^ ff.fw[1]);
+            break;
 	}
-
     case MAKE_HASH_CDR_PRE_OP:
 	term = (Eterm) WSTACK_POP(stack);
 	if (is_not_list(term)) {
@@ -1474,6 +1476,9 @@ make_hash2(Eterm term)
 	    {
 		FloatDef ff;
 		GET_DOUBLE(term, ff);
+                if (ff.fd == 0.0f) {
+                    ff.fd = 0.0f; /* ensure pos. 0.0 */
+                }
 #if defined(WORDS_BIGENDIAN) || defined(DOUBLE_MIDDLE_ENDIAN)
 		UINT32_HASH_2(ff.fw[0], ff.fw[1], HCONST_12);
 #else
@@ -1893,8 +1898,8 @@ make_internal_hash(Eterm term)
 	    {
 		FloatDef ff;
 		GET_DOUBLE(term, ff);
-                if (ff.fd == 0.0) {
-                    ff.fd = 0.0; /* ensure pos. 0.0 */
+                if (ff.fd == 0.0f) {
+                    ff.fd = 0.0f; /* ensure pos. 0.0 */
                 }
 		UINT32_HASH_2(ff.fw[0], ff.fw[1], HCONST_12);
 		goto pop_next;
@@ -2079,12 +2084,14 @@ tail_recur:
 	break;
     case FLOAT_DEF: 
 	{
-	    FloatDef ff;
-	    GET_DOUBLE(term, ff);
-	    hash = hash*FUNNY_NUMBER6 + (ff.fw[0] ^ ff.fw[1]);
+            FloatDef ff;
+            GET_DOUBLE(term, ff);
+            if (ff.fd == 0.0f) {
+                ff.fd = 0.0f; /* ensure pos. 0.0 */
+            }
+            hash = hash*FUNNY_NUMBER6 + (ff.fw[0] ^ ff.fw[1]);
 	}
 	break;
-
     case MAKE_HASH_CDR_PRE_OP:
 	term = (Eterm) WSTACK_POP(stack);
 	if (is_not_list(term)) {
diff --git a/erts/emulator/test/hash_SUITE.erl b/erts/emulator/test/hash_SUITE.erl
index 647bb45049..5fa45f772d 100644
--- a/erts/emulator/test/hash_SUITE.erl
+++ b/erts/emulator/test/hash_SUITE.erl
@@ -73,6 +73,7 @@ config(priv_dir,_) ->
 	 init_per_group/2,end_per_group/2,
 	 test_basic/1,test_cmp/1,test_range/1,test_spread/1,
 	 test_phash2/1,otp_5292/1,bit_level_binaries/1,otp_7127/1,
+         test_hash_zero/1,
 	 end_per_testcase/2,init_per_testcase/2]).
 init_per_testcase(_Case, Config) ->
     Dog=test_server:timetrap(test_server:minutes(10)),
@@ -86,7 +87,9 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
     [test_basic, test_cmp, test_range, test_spread,
-     test_phash2, otp_5292, bit_level_binaries, otp_7127].
+     test_phash2, otp_5292, bit_level_binaries, otp_7127,
+     test_hash_zero
+    ].
 
 groups() -> 
     [].
@@ -160,6 +163,8 @@ otp_7127(doc) ->
 otp_7127(Config) when is_list(Config) ->
     otp_7127_test().
 
+test_hash_zero(Config) when is_list(Config) ->
+    hash_zero_test().
 -endif.
 
 
@@ -591,6 +596,26 @@ otp_7127_test() ->
     38990304 = erlang:phash2(<<"Scott9">>),
     ok.
 
+hash_zero_test() ->
+    Zs = [0.0, -0.0, 0/-1, 0.0/-1, 0/-(1 bsl 65),
+          binary_to_term(<<131,70,0,0,0,0,0,0,0,0>>),    %% +0.0
+          binary_to_term(<<131,70,128,0,0,0,0,0,0,0>>)], %% -0.0
+    ok = hash_zero_test(Zs,fun(T) -> erlang:phash2(T, 1 bsl 32) end),
+    ok = hash_zero_test(Zs,fun(T) -> erlang:phash(T, 1 bsl 32) end),
+    ok = hash_zero_test(Zs,fun(T) -> erlang:hash(T, (1 bsl 27) - 1) end),
+    ok.
+
+hash_zero_test([Z|Zs],F) ->
+    hash_zero_test(Zs,Z,F(Z),F).
+hash_zero_test([Z|Zs],Z0,V,F) ->
+    true = Z0 =:= Z, %% assert exact equal
+    Z0   = Z,        %% assert matching
+    V    = F(Z),     %% assert hash
+    hash_zero_test(Zs,Z0,V,F);
+hash_zero_test([],_,_,_) ->
+    ok.
+
+
 %%
 %% Reference implementation of integer hashing
 %%
-- 
cgit v1.2.3