Merge branch 'bjorn/hipe/fix-race-condition'

* bjorn/hipe/fix-race-condition:
  Delay patching of closures to eliminate a race condition
  hipe: Break apart hipe_bif:make_fe/3 into two BIFs

3 files changed, 49 insertions, 17 deletions

diff --git a/erts/emulator/hipe/hipe_bif0.c b/erts/emulator/hipe/hipe_bif0.c
index fa99c817f0..2497d51df1 100644
--- a/erts/emulator/hipe/hipe_bif0.c
+++ b/erts/emulator/hipe/hipe_bif0.c
@@ -1101,9 +1101,9 @@ BIF_RETTYPE hipe_bifs_make_fun_3(BIF_ALIST_3)
 #endif
 
 /*
- * args: Nativecodeaddress, Module, {Uniq, Index, BeamAddress}
+ * args: Module, {Uniq, Index, BeamAddress}
  */
-BIF_RETTYPE hipe_bifs_make_fe_3(BIF_ALIST_3)
+BIF_RETTYPE hipe_bifs_get_fe_2(BIF_ALIST_2)
 {
     Eterm mod;
     Uint index;
@@ -1111,20 +1111,15 @@ BIF_RETTYPE hipe_bifs_make_fe_3(BIF_ALIST_3)
     void *beam_address;
     ErlFunEntry *fe;
     Eterm *tp;
-    void *native_address;
-
-    native_address = term_to_address(BIF_ARG_1);
-    if (!native_address)
-	BIF_ERROR(BIF_P, BADARG);
 
-    if (is_not_atom(BIF_ARG_2))
+    if (is_not_atom(BIF_ARG_1))
 	BIF_ERROR(BIF_P, BADARG);
-    mod = BIF_ARG_2;
+    mod = BIF_ARG_1;
 
-    if (is_not_tuple(BIF_ARG_3) ||
-	(arityval(*tuple_val(BIF_ARG_3)) != 3))
+    if (is_not_tuple(BIF_ARG_2) ||
+	(arityval(*tuple_val(BIF_ARG_2)) != 3))
 	BIF_ERROR(BIF_P, BADARG);
-    tp = tuple_val(BIF_ARG_3);
+    tp = tuple_val(BIF_ARG_2);
     if (term_to_Uint(tp[1], &uniq) == 0)
 	BIF_ERROR(BIF_P, BADARG);
     if (term_to_Uint(tp[2], &index) == 0)
@@ -1144,10 +1139,28 @@ BIF_RETTYPE hipe_bifs_make_fe_3(BIF_ALIST_3)
 	printf("no fun entry for %s %ld:%ld\n", atom_buf, uniq, index);
 	BIF_ERROR(BIF_P, BADARG);
     }
+    BIF_RET(address_to_term((void *)fe, BIF_P));
+}
+
+/*
+ * args: FE, Nativecodeaddress
+ */
+BIF_RETTYPE hipe_bifs_set_native_address_in_fe_2(BIF_ALIST_2)
+{
+    ErlFunEntry *fe;
+    void *native_address;
+
+    fe = (ErlFunEntry *)term_to_address(BIF_ARG_1);
+    if (!fe)
+	BIF_ERROR(BIF_P, BADARG);
+    native_address = term_to_address(BIF_ARG_2);
+    if (!native_address)
+	BIF_ERROR(BIF_P, BADARG);
+
     fe->native_address = native_address;
     if (erts_refc_dectest(&fe->refc, 0) == 0)
 	erts_erase_fun_entry(fe);
-    BIF_RET(address_to_term((void *)fe, BIF_P));
+    BIF_RET(am_true);
 }
 
 #if 0 /* XXX: unused */
diff --git a/erts/emulator/hipe/hipe_bif0.tab b/erts/emulator/hipe/hipe_bif0.tab
index ce641365e9..2514b1c3a5 100644
--- a/erts/emulator/hipe/hipe_bif0.tab
+++ b/erts/emulator/hipe/hipe_bif0.tab
@@ -69,7 +69,8 @@ bif hipe_bifs:atom_to_word/1
 bif hipe_bifs:term_to_word/1
 
 #bif hipe_bifs:make_fun/3
-bif hipe_bifs:make_fe/3
+bif hipe_bifs:get_fe/2
+bif hipe_bifs:set_native_address_in_fe/2
 
 #bif hipe_bifs:make_native_stub/2
 bif hipe_bifs:find_na_or_make_stub/2
diff --git a/lib/kernel/src/hipe_unified_loader.erl b/lib/kernel/src/hipe_unified_loader.erl
index e111cb800e..976d5e35cb 100644
--- a/lib/kernel/src/hipe_unified_loader.erl
+++ b/lib/kernel/src/hipe_unified_loader.erl
@@ -203,6 +203,7 @@ load_common(Mod, Bin, Beam, OldReferencesToPatch) ->
 	   "please regenerate native code for this runtime system\n", [Mod]),
       bad_crc;
     true ->
+      put(closures_to_patch, []),
       %% Create data segment
       {ConstAddr,ConstMap2} =
 	create_data_segment(ConstAlign, ConstSize, ConstMap),
@@ -224,14 +225,23 @@ load_common(Mod, Bin, Beam, OldReferencesToPatch) ->
       %% Patch all dynamic references in the code.
       %%  Function calls, Atoms, Constants, System calls
       ok = patch(Refs, CodeAddress, ConstMap2, Addresses, TrampolineMap),
+
       %% Tell the system where the loaded funs are. 
       %%  (patches the BEAM code to redirect to native.)
       case Beam of
 	[] ->
+	  %% This module was previously loaded as BEAM code during system
+	  %% start-up before the code server has started (-enable-native-libs
+	  %% is active), so we must now patch the pre-existing entries in the
+	  %% fun table with the native code addresses for all closures.
+	  lists:foreach(fun({FE, DestAddress}) ->
+			    hipe_bifs:set_native_address_in_fe(FE, DestAddress)
+			end, erase(closures_to_patch)),
 	  export_funs(Addresses),
 	  ok;
 	BeamBinary when is_binary(BeamBinary) ->
 	  %% Find all closures in the code.
+	  [] = erase(closures_to_patch),	%Clean up, assertion.
 	  ClosurePatches = find_closure_patches(Refs),
 	  AddressesOfClosuresToPatch =
 	    calculate_addresses(ClosurePatches, CodeAddress, Addresses),
@@ -245,6 +255,9 @@ load_common(Mod, Bin, Beam, OldReferencesToPatch) ->
       %% The call to export_funs/1 above updated the native addresses
       %% for the targets, so passing 'Addresses' is not needed.
       redirect(ReferencesToPatch),
+      %% Final clean up.
+      _ = erase(hipe_patch_closures),
+      _ = erase(hipe_assert_code_area),
       ?debug_msg("****************Loader Finished****************\n", []),
       {module,Mod}  % for compatibility with code:load_file/1
   end.
@@ -562,12 +575,17 @@ patch_closure(DestMFA, Uniq, Index, Address, Addresses) ->
   case get(hipe_patch_closures) of
     false ->
       []; % This is taken care of when registering the module.
-    true -> % We are not loading a module patch these closures
+    true ->
+      %% We are replacing a previosly loaded BEAM module with native code,
+      %% so we must reference the pre-existing entries in the fun table
+      %% from the native code. We must delay actually patching the native
+      %% address into the fun entry to ensure that the native code cannot
+      %% be called until it has been completely fixed up.
       RemoteOrLocal = local, % closure code refs are local
       DestAddress = get_native_address(DestMFA, Addresses, RemoteOrLocal),
       BEAMAddress = hipe_bifs:fun_to_address(DestMFA),
-      FE = hipe_bifs:make_fe(DestAddress, mod(DestMFA), 
-			     {Uniq, Index, BEAMAddress}),
+      FE = hipe_bifs:get_fe(mod(DestMFA), {Uniq, Index, BEAMAddress}),
+      put(closures_to_patch, [{FE,DestAddress}|get(closures_to_patch)]),
       ?debug_msg("Patch FE(~w) to 0x~.16b->0x~.16b (emu:0x~.16b)\n",
 		 [DestMFA, FE, DestAddress, BEAMAddress]),
       ?ASSERT(assert_local_patch(Address)),