From 81c7db0e247a6ee1b7a5c2764aa9575d7f6fb88a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 28 Feb 2014 06:08:51 +0100 Subject: hipe: Break apart hipe_bif:make_fe/3 into two BIFs This commit is a preparation for eliminating a race condition loading the native code for modules whose BEAM code has already been loaded. Here we introduce two new BIFs so that looking up a fun entry is separate from setting the native address in the fun entry. --- lib/kernel/src/hipe_unified_loader.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/kernel/src/hipe_unified_loader.erl b/lib/kernel/src/hipe_unified_loader.erl index e111cb800e..fd55b54d1f 100644 --- a/lib/kernel/src/hipe_unified_loader.erl +++ b/lib/kernel/src/hipe_unified_loader.erl @@ -566,8 +566,8 @@ patch_closure(DestMFA, Uniq, Index, Address, Addresses) -> 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}), + hipe_bifs:set_native_address_in_fe(FE, DestAddress), ?debug_msg("Patch FE(~w) to 0x~.16b->0x~.16b (emu:0x~.16b)\n", [DestMFA, FE, DestAddress, BEAMAddress]), ?ASSERT(assert_local_patch(Address)), -- cgit v1.2.3 From 63a653050b5e6ca28cde1e8616c840abafe71100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 28 Feb 2014 06:34:43 +0100 Subject: Delay patching of closures to eliminate a race condition The loader of native code tries to avoid race condition by bringing down the system to a single scheduler. Unfortunately, that will not completely avoid race conditions beacuse other processes may still run while the code is being loaded. Therefore, we must still make sure that native code has been fully fixed up before it can be executed. Under unlucky circumstances, it was possible for the global name register process to call a fun in native code before the native code for the fun had been fully fixed up. The run-time system would crash when running the not-fully-fixed-up code. Here is a way to make the bug 100% reproducible. First add this function to the 'global' module: silly_looper(List) -> lists:foreach(fun(E) -> put(any_atom, E) end, List), silly_looper(List). Then insert the following code at the beginning of the init/1 function in the 'global' module: spawn_link(fun() -> silly_looper(lists:seq(1, 100)) end), The run-time system will crash during start up when the native code for the 'global' module is being loaded. What will happen is that lists:foreach/2 (which runs in native code) will call the native code for the fun in silly_looper/1 before the reference to 'any_atom' has been changed from a 0 to the proper tagged atom value. The put/2 BIF will exit when attempting to calculate the hash value for the illegal value 0 (it is not properly tagged). To eliminate this race condition, we must delay patching the native code addresses for a fun into the fun entry until the native code has been fully fixed up. We will use the new BIFs introduced in the previous commit. While we are at it, we will also make sure that we erase any temporary variables in the process dictionary used by the hipe_unified_loader module. --- lib/kernel/src/hipe_unified_loader.erl | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/kernel/src/hipe_unified_loader.erl b/lib/kernel/src/hipe_unified_loader.erl index fd55b54d1f..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:get_fe(mod(DestMFA), {Uniq, Index, BEAMAddress}), - hipe_bifs:set_native_address_in_fe(FE, DestAddress), + 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)), -- cgit v1.2.3