Merge branch 'bjorn/better-fun-info/OTP-15837'

* bjorn/better-fun-info/OTP-15837:
  Create a shared wrapper function for all occurrences of 'fun F/A'
  Support sharing of fun entries in the runtime system
  erl_fun.c: Remove unused struct definition
  hipe: Use the new index when translating funs
  Stop supporting decoding of old funs in the external term format
  genop.tab: Insert an "OTP 23" comment for clarity

6 files changed, 81 insertions, 135 deletions

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 0de694f449..9add87d944 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -315,6 +315,7 @@ typedef struct LoaderState {
 				 * (or 0 if there is no on_load function)
 				 */
     int otp_20_or_higher;       /* Compiled with OTP 20 or higher */
+    unsigned max_opcode;        /* Highest opcode used in module */
 
     /*
      * Atom table.
@@ -1588,6 +1589,17 @@ static int
 read_lambda_table(LoaderState* stp)
 {
     unsigned int i;
+    unsigned int otp_22_or_lower;
+
+    /*
+     * Determine whether this module was compiled with OTP 22 or lower
+     * by looking at the max opcode number. The compiler in OTP 23 will
+     * always set the max opcode to the opcode for `swap` (whether
+     * actually used or not) so that a module compiled for OTP 23
+     * cannot be loaded in earlier versions.
+     */
+
+    otp_22_or_lower = stp->max_opcode < genop_swap_2;
 
     GetInt(stp, 4, stp->num_lambdas);
     if (stp->num_lambdas > stp->lambdas_allocated) {
@@ -1619,6 +1631,29 @@ read_lambda_table(LoaderState* stp)
 	GetInt(stp, 4, Index);
 	GetInt(stp, 4, stp->lambdas[i].num_free);
 	GetInt(stp, 4, OldUniq);
+
+        /*
+         * Fun entries are now keyed by the explicit ("new") index in
+         * the fun entry. That allows multiple make_fun2 instructions
+         * to share the same fun entry (when the `fun F/A` syntax is
+         * used). Before OTP 23, fun entries were keyed by the old
+         * index, which is the order of the entries in the fun
+         * chunk. Each make_fun2 needed to refer to its own fun entry.
+         *
+         * Modules compiled before OTP 23 can safely be loaded if the
+         * old index and the new index are equal. That is true for all
+         * modules compiled with OTP R15 and later.
+         */
+        if (otp_22_or_lower && i != Index) {
+            /*
+             * Compiled with a compiler before OTP R15B. The new indices
+             * are not reliable, so it is not safe to load this module.
+             */
+            LoadError2(stp, "please re-compile this module with an "
+                       ERLANG_OTP_RELEASE " compiler "
+                       "(old-style fun with indices: %d/%d)",
+                       i, Index);
+        }
 	fe = erts_put_fun_entry2(stp->module, OldUniq, i, stp->mod_md5,
 				 Index, arity-stp->lambdas[i].num_free);
 	stp->lambdas[i].fe = fe;
@@ -1839,7 +1874,6 @@ read_code_header(LoaderState* stp)
 {
     unsigned head_size;
     unsigned version;
-    unsigned opcode_max;
     int i;
 
     /*
@@ -1871,8 +1905,8 @@ read_code_header(LoaderState* stp)
     /*
      * Verify the number of the highest opcode used.
      */
-    GetInt(stp, 4, opcode_max);
-    if (opcode_max > MAX_GENERIC_OPCODE) {
+    GetInt(stp, 4, stp->max_opcode);
+    if (stp->max_opcode > MAX_GENERIC_OPCODE) {
 	LoadError2(stp,
 		   "This BEAM file was compiled for a later version"
 		   " of the run-time system than " ERLANG_OTP_RELEASE ".\n"
@@ -1880,7 +1914,7 @@ read_code_header(LoaderState* stp)
 		   ERLANG_OTP_RELEASE " compiler.\n"
 		   "  (Use of opcode %d; this emulator supports "
 		   "only up to %d.)",
-		   opcode_max, MAX_GENERIC_OPCODE);
+		   stp->max_opcode, MAX_GENERIC_OPCODE);
     }
 
     GetInt(stp, 4, stp->num_labels);
diff --git a/erts/emulator/beam/erl_fun.c b/erts/emulator/beam/erl_fun.c
index 9c866250bb..257f9bf5b3 100644
--- a/erts/emulator/beam/erl_fun.c
+++ b/erts/emulator/beam/erl_fun.c
@@ -100,27 +100,6 @@ int erts_fun_table_sz(void)
 }
 
 ErlFunEntry*
-erts_put_fun_entry(Eterm mod, int uniq, int index)
-{
-    ErlFunEntry template;
-    ErlFunEntry* fe;
-    erts_aint_t refc;
-    ASSERT(is_atom(mod));
-    template.old_uniq = uniq;
-    template.old_index = index;
-    template.module = mod;
-    erts_fun_write_lock();
-    fe = (ErlFunEntry *) hash_put(&erts_fun_table, (void*) &template);
-    sys_memset(fe->uniq, 0, sizeof(fe->uniq));
-    fe->index = 0;
-    refc = erts_refc_inctest(&fe->refc, 0);
-    if (refc < 2) /* New or pending delete */
-	erts_refc_inc(&fe->refc, 1);
-    erts_fun_write_unlock();
-    return fe;
-}
-
-ErlFunEntry*
 erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index,
 		    byte* uniq, int index, int arity)
 {
@@ -130,12 +109,12 @@ erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index,
 
     ASSERT(is_atom(mod));
     template.old_uniq = old_uniq;
-    template.old_index = old_index;
+    template.index = index;
     template.module = mod;
     erts_fun_write_lock();
     fe = (ErlFunEntry *) hash_put(&erts_fun_table, (void*) &template);
     sys_memcpy(fe->uniq, uniq, sizeof(fe->uniq));
-    fe->index = index;
+    fe->old_index = old_index;
     fe->arity = arity;
     refc = erts_refc_inctest(&fe->refc, 0);
     if (refc < 2) /* New or pending delete */
@@ -144,13 +123,6 @@ erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index,
     return fe;
 }
 
-struct my_key {
-    Eterm mod;
-    byte* uniq;
-    int index;
-    ErlFunEntry* fe;
-};
-
 ErlFunEntry*
 erts_get_fun_entry(Eterm mod, int uniq, int index)
 {
@@ -159,7 +131,7 @@ erts_get_fun_entry(Eterm mod, int uniq, int index)
 
     ASSERT(is_atom(mod));
     template.old_uniq = uniq;
-    template.old_index = index;
+    template.index = index;
     template.module = mod;
     erts_fun_read_lock();
     ret = (ErlFunEntry *) hash_get(&erts_fun_table, (void*) &template);
@@ -315,15 +287,27 @@ erts_dump_fun_entries(fmtfn_t to, void *to_arg)
 static HashValue
 fun_hash(ErlFunEntry* obj)
 {
-    return (HashValue) (obj->old_uniq ^ obj->old_index ^ atom_val(obj->module));
+    return (HashValue) (obj->old_uniq ^ obj->index ^ atom_val(obj->module));
 }
 
 static int
 fun_cmp(ErlFunEntry* obj1, ErlFunEntry* obj2)
 {
-    return !(obj1->module == obj2->module && 
+    /*
+     * OTP 23: Use 'index' (instead of 'old_index') when comparing fun
+     * entries. In OTP 23, multiple make_fun2 instructions may refer to the
+     * the same 'index' (for the wrapper function generated for the
+     * 'fun F/A' syntax).
+     *
+     * This is safe when loading code compiled with OTP R15 and later,
+     * because since R15 (2011), the 'index' has been reliably equal
+     * to 'old_index'. The loader refuses to load modules compiled before
+     * OTP R15.
+     */
+
+    return !(obj1->module == obj2->module &&
 	     obj1->old_uniq == obj2->old_uniq &&
-	     obj1->old_index == obj2->old_index);
+	     obj1->index == obj2->index);
 }
 
 static ErlFunEntry*
@@ -333,7 +317,7 @@ fun_alloc(ErlFunEntry* template)
 						  sizeof(ErlFunEntry));
 
     obj->old_uniq = template->old_uniq;
-    obj->old_index = template->old_index;
+    obj->index = template->index;
     obj->module = template->module;
     erts_refc_init(&obj->refc, -1);
     obj->address = unloaded_fun;
diff --git a/erts/emulator/beam/erl_fun.h b/erts/emulator/beam/erl_fun.h
index fb2901d866..eefc7a95bb 100644
--- a/erts/emulator/beam/erl_fun.h
+++ b/erts/emulator/beam/erl_fun.h
@@ -74,7 +74,6 @@ void erts_init_fun_table(void);
 void erts_fun_info(fmtfn_t, void *);
 int erts_fun_table_sz(void);
 
-ErlFunEntry* erts_put_fun_entry(Eterm mod, int uniq, int index);
 ErlFunEntry* erts_get_fun_entry(Eterm mod, int uniq, int index);
 
 ErlFunEntry* erts_put_fun_entry2(Eterm mod, int old_uniq, int old_index,
diff --git a/erts/emulator/beam/external.c b/erts/emulator/beam/external.c
index ec67ab2aed..8a8e62a608 100644
--- a/erts/emulator/beam/external.c
+++ b/erts/emulator/beam/external.c
@@ -4011,73 +4011,6 @@ dec_term_atom_common:
 		next = &(funp->creator);
 		break;
 	    }
-	case FUN_EXT:
-	    {
-		ErlFunThing* funp = (ErlFunThing *) hp;
-		Eterm module;
-		Sint old_uniq;
-		Sint old_index;
-		unsigned num_free;
-		int i;
-		Eterm temp;
-
-		num_free = get_int32(ep);
-		ep += 4;
-		hp += ERL_FUN_SIZE;
-		hp += num_free;
-		factory->hp = hp;
-		funp->thing_word = HEADER_FUN;
-		funp->num_free = num_free;
-		*objp = make_fun(funp);
-
-		/* Creator pid */
-		if ((*ep != PID_EXT && *ep != NEW_PID_EXT)
-		    || (ep = dec_pid(edep, factory, ep+1,
-				     &funp->creator, *ep))==NULL) {
-		    goto error;
-		}
-
-		/* Module */
-		if ((ep = dec_atom(edep, ep, &module)) == NULL) {
-		    goto error;
-		}
-
-		/* Index */
-		if ((ep = dec_term(edep, factory, ep, &temp, NULL)) == NULL) {
-		    goto error;
-		}
-		if (!is_small(temp)) {
-		    goto error;
-		}
-		old_index = unsigned_val(temp);
-
-		/* Uniq */
-		if ((ep = dec_term(edep, factory, ep, &temp, NULL)) == NULL) {
-		    goto error;
-		}
-		if (!is_small(temp)) {
-		    goto error;
-		}
-		
-		/*
-		 * It is safe to link the fun into the fun list only when
-		 * no more validity tests can fail.
-		 */
-		funp->next = factory->off_heap->first;
-		factory->off_heap->first = (struct erl_off_heap_header*)funp;
-		old_uniq = unsigned_val(temp);
-
-		funp->fe = erts_put_fun_entry(module, old_uniq, old_index);
-		funp->arity = funp->fe->address[-1] - num_free;
-		hp = factory->hp;
-
-		/* Environment */
-		for (i = num_free-1; i >= 0; i--) {
-		    funp->env[i] = (Eterm) next;
-		    next = funp->env + i;
-		}
-		break;
-	    }
 	case ATOM_INTERNAL_REF2:
 	    n = get_int16(ep);
 	    ep += 2;
@@ -4836,9 +4769,6 @@ init_done:
 		total_size = get_int32(ep);
 		CHKSIZE(total_size);		
 		ep += 1+16+4+4;
-		/*FALLTHROUGH*/
-
-	    case FUN_EXT:
 		CHKSIZE(4);
 		num_free = get_int32(ep);
 		ep += 4;
@@ -4849,6 +4779,12 @@ init_done:
 		heap_size += ERL_FUN_SIZE + num_free;
 		break;
 	    }
+	case FUN_EXT:
+            /*
+             * OTP 23: No longer support decoding the old fun
+             * representation.
+             */
+            goto error;
 	case ATOM_INTERNAL_REF2:
 	    SKIP(2+atom_extra_skip);
 	    atom_extra_skip = 0;
diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c
index 88cdcc2675..ad71828d72 100644
--- a/erts/emulator/beam/utils.c
+++ b/erts/emulator/beam/utils.c
@@ -907,7 +907,7 @@ tail_recur:
 	    hash = hash * FUNNY_NUMBER10 + num_free;
 	    hash = hash*FUNNY_NUMBER1 +
 		(atom_tab(atom_val(funp->fe->module))->slot.bucket.hvalue);
-	    hash = hash*FUNNY_NUMBER2 + funp->fe->old_index;
+	    hash = hash*FUNNY_NUMBER2 + funp->fe->index;
 	    hash = hash*FUNNY_NUMBER2 + funp->fe->old_uniq;
 	    if (num_free > 0) {
 		if (num_free > 1) {
@@ -1636,7 +1636,7 @@ make_hash2_helper(Eterm term_param, const int can_trap, Eterm* state_mref_write_
 		     atom_tab(atom_val(funp->fe->module))->slot.bucket.hvalue,
 		     HCONST);
 		UINT32_HASH_2
-		    (funp->fe->old_index, funp->fe->old_uniq, HCONST);
+		    (funp->fe->index, funp->fe->old_uniq, HCONST);
 		if (ctx.num_free == 0) {
 		    goto hash2_common;
 		} else {
@@ -2160,7 +2160,7 @@ make_internal_hash(Eterm term, Uint32 salt)
 		ErlFunThing* funp = (ErlFunThing *) fun_val(term);
 		Uint num_free = funp->num_free;
                 UINT32_HASH_2(num_free, funp->fe->module, HCONST_20);
-                UINT32_HASH_2(funp->fe->old_index, funp->fe->old_uniq, HCONST_21);
+                UINT32_HASH_2(funp->fe->index, funp->fe->old_uniq, HCONST_21);
 		if (num_free == 0) {
 		    goto pop_next;
 		} else {
@@ -2810,7 +2810,7 @@ tailrecur_ne:
 		    f1 = (ErlFunThing *) fun_val(a);
 		    f2 = (ErlFunThing *) fun_val(b);
 		    if (f1->fe->module != f2->fe->module ||
-			f1->fe->old_index != f2->fe->old_index ||
+			f1->fe->index != f2->fe->index ||
 			f1->fe->old_uniq != f2->fe->old_uniq ||
 			f1->num_free != f2->num_free) {
 			goto not_equal;
@@ -3405,7 +3405,7 @@ tailrecur_ne:
 		    if (diff != 0) {
 			RETURN_NEQ(diff);
 		    }
-		    diff = f1->fe->old_index - f2->fe->old_index;
+		    diff = f1->fe->index - f2->fe->index;
 		    if (diff != 0) {
 			RETURN_NEQ(diff);
 		    }
diff --git a/erts/emulator/test/hash_SUITE.erl b/erts/emulator/test/hash_SUITE.erl
index 1bf9e033bf..dd71c3da58 100644
--- a/erts/emulator/test/hash_SUITE.erl
+++ b/erts/emulator/test/hash_SUITE.erl
@@ -223,26 +223,17 @@ basic_test() ->
 				    16#77777777777777],16#FFFFFFFF),
     ExternalReference = <<131,114,0,3,100,0,13,110,111,110,111,100,101,64,
 			 110,111,104,111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0>>,
-    1113403635 = erlang:phash(binary_to_term(ExternalReference),
-				    16#FFFFFFFF),
-    ExternalFun = <<131,117,0,0,0,3,103,100,0,13,110,111,110,111,100,101,64,
-		   110,111,104,111,115,116,0,0,0,38,0,0,0,0,0,100,0,8,101,
-		   114,108,95,101,118,97,108,97,20,98,5,182,139,98,108,0,0,
-		   0,3,104,2,100,0,1,66,109,0,0,0,33,131,114,0,3,100,0,13,
-		   110,111,110,111,100,101,64,110,111,104,111,115,116,0,0,
-		   0,0,122,0,0,0,0,0,0,0,0,104,2,100,0,1,76,107,0,33,131,
-		   114,0,3,100,0,13,110,111,110,111,100,101,64,110,111,104,
-		   111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0,104,2,100,0,1,82,
-		   114,0,3,100,0,13,110,111,110,111,100,101,64,110,111,104,
-		   111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0,106,108,0,0,0,1,
-		   104,5,100,0,6,99,108,97,117,115,101,97,1,106,106,108,0,0,
-		   0,1,104,3,100,0,7,105,110,116,101,103,101,114,97,1,97,1,
-		   106,106,104,3,100,0,4,101,118,97,108,104,2,100,0,5,115,
-		   104,101,108,108,100,0,10,108,111,99,97,108,95,102,117,
-		   110,99,108,0,0,0,1,103,100,0,13,110,111,110,111,100,101,
-		   64,110,111,104,111,115,116,0,0,0,22,0,0,0,0,0,106>>,
-    170987488 = erlang:phash(binary_to_term(ExternalFun),
-				   16#FFFFFFFF),
+    ExternalReference = <<131,114,0,3,100,0,13,110,111,110,111,100,101,64,
+			 110,111,104,111,115,116,0,0,0,0,122,0,0,0,0,0,0,0,0>>,
+    1113403635 = phash_from_external(ExternalReference),
+
+    ExternalFun = <<131,112,0,0,0,70,1,212,190,220,28,179,144,194,131,
+                    19,215,105,97,77,251,125,93,0,0,0,0,0,0,0,2,100,0,1,
+                    116,97,0,98,6,165,246,224,103,100,0,13,110,111,
+                    110,111,100,101,64,110,111,104,111,115,116,0,0,0,91,
+                    0,0,0,0,0,97,2,97,1>>,
+    25769064 = phash_from_external(ExternalFun),
+
     case (catch erlang:phash(1,0)) of
 	{'EXIT',{badarg, _}} ->
 	    ok;
@@ -250,6 +241,8 @@ basic_test() ->
 	    exit(phash_accepted_zero_as_range)
     end.
 
+phash_from_external(Ext) ->
+    erlang:phash(binary_to_term(Ext), 16#FFFFFFFF).
 
 range_test() ->
     F = fun(From,From,_FF) ->