[PATCH] Fix hang when calling functions in a module with an on_load attribute from a native module

pguyot@REDACTED pguyot@REDACTED
Wed Sep 22 15:38:25 CEST 2010


From: Paul Guyot <pguyot@REDACTED>

The fix consists in adding a new opcode for hipe stub calls that
is used to replace the call to error_handler:undefined_function/3. This
stub directly calls the function: there is no need to go through
error_handler:undefined_function/3 once we know the module is loaded,
and, more importantly, when error_handler module itself was natively
compiled (with native code loaded), the emulator did loop infinitely.

The fix also comes with a non-regression test that will prevent this
bug to reappear again.
---
 erts/emulator/beam/beam_bif_load.c                 |    9 ++++-
 erts/emulator/beam/beam_emu.c                      |   18 ++++++++++
 erts/emulator/beam/beam_load.h                     |    1 +
 erts/emulator/beam/export.h                        |    7 ++--
 erts/emulator/test/Makefile                        |    1 +
 erts/emulator/test/hipe_SUITE.erl                  |   35 ++++++++++++++++++++
 erts/emulator/test/hipe_SUITE_data/beam_callee.erl |   10 ++++++
 .../test/hipe_SUITE_data/native_caller.erl         |    9 +++++
 8 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 erts/emulator/test/hipe_SUITE.erl
 create mode 100644 erts/emulator/test/hipe_SUITE_data/beam_callee.erl
 create mode 100644 erts/emulator/test/hipe_SUITE_data/native_caller.erl

diff --git a/erts/emulator/beam/beam_bif_load.c b/erts/emulator/beam/beam_bif_load.c
index 6ae9736..72864dd 100644
--- a/erts/emulator/beam/beam_bif_load.c
+++ b/erts/emulator/beam/beam_bif_load.c
@@ -336,8 +336,15 @@ BIF_RETTYPE finish_after_on_load_2(BIF_ALIST_2)
 	    if (ep != NULL &&
 		ep->code[0] == BIF_ARG_1 &&
 		ep->code[4] != 0) {
+		/* the fixed address is in ep->code[4], see final_touch in beam_load */
 		ep->address = (void *) ep->code[4];
-		ep->code[4] = 0;
+		/* any hipe stub to this function points to ep->code[3], which in turn
+		currently is em_call_error_handler -- to load the module through
+		error_handler:undefined_function/3. Since the function is
+		loaded, we can jump to it directly with em_call_from_hipe_stub, and
+		this would actually prevent any infinite loop if error_handler is
+		actually natively compiled. */
+		ep->code[3] = em_call_from_hipe_stub;
 	    }
 	}
 	modp->code[MI_ON_LOAD_FUNCTION_PTR] = 0;
diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index 8a0e12d..73b9ebb 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -221,6 +221,7 @@ BeamInstr beam_continue_exit[1];
 BeamInstr* em_call_error_handler;
 BeamInstr* em_apply_bif;
 BeamInstr* em_call_traced_function;
+BeamInstr* em_call_from_hipe_stub;
 
 
 /* NOTE These should be the only variables containing trace instructions.
@@ -3043,6 +3044,22 @@ void process_main(void)
      goto post_error_handling;
  }
 
+    /*
+     * At this point, I points to the code[3] in the export entry for
+     * a loaded function with a hipe stub.
+     *
+     * code[0]: Module
+     * code[1]: Function
+     * code[2]: Arity
+     * code[3]: &&call_from_hipe_stub
+     * code[4]: Address of function.
+     */
+ OpCase(call_from_hipe_stub): {
+     /* We might want to fix the stub (?) to earn few cycles */
+     SET_I((BeamInstr *)Arg(0));
+     Goto(*I);
+ }
+
  OpCase(call_error_handler):
     /*
      * At this point, I points to the code[3] in the export entry for
@@ -5034,6 +5051,7 @@ apply_bif_or_nif_epilogue:
      em_call_error_handler = OpCode(call_error_handler);
      em_call_traced_function = OpCode(call_traced_function);
      em_apply_bif = OpCode(apply_bif);
+     em_call_from_hipe_stub = OpCode(call_from_hipe_stub);
 
      beam_apply[0]             = (BeamInstr) OpCode(i_apply);
      beam_apply[1]             = (BeamInstr) OpCode(normal_exit);
diff --git a/erts/emulator/beam/beam_load.h b/erts/emulator/beam/beam_load.h
index 26e3054..373d242 100644
--- a/erts/emulator/beam/beam_load.h
+++ b/erts/emulator/beam/beam_load.h
@@ -48,6 +48,7 @@ extern BeamInstr beam_debug_apply[];
 extern BeamInstr* em_call_error_handler;
 extern BeamInstr* em_apply_bif;
 extern BeamInstr* em_call_traced_function;
+extern BeamInstr* em_call_from_hipe_stub;
 typedef struct {
     BeamInstr* start;		/* Pointer to start of module. */
     BeamInstr* end;			/* Points one word beyond last function in module. */
diff --git a/erts/emulator/beam/export.h b/erts/emulator/beam/export.h
index c604fdf..b472eb7 100644
--- a/erts/emulator/beam/export.h
+++ b/erts/emulator/beam/export.h
@@ -45,11 +45,12 @@ typedef struct export
      * code[3]: This entry is 0 unless the 'address' field points to it.
      *          Threaded code instruction to load function
      *		(em_call_error_handler), execute BIF (em_apply_bif,
-     *		em_apply_apply), or call a traced function
-     *		(em_call_traced_function).
+     *		em_apply_apply), call a traced function
+     *		(em_call_traced_function) or call a loaded function (for hipe stub
+     *      of on_load modules).
      * code[4]: Function pointer to BIF function (for BIFs only)
      *		or pointer to threaded code if the module has an
-     *		on_load function that has not been run yet.
+     *		on_load function.
      *		Otherwise: 0.
      */
     BeamInstr code[5];
diff --git a/erts/emulator/test/Makefile b/erts/emulator/test/Makefile
index a4c02da..c52d0d5 100644
--- a/erts/emulator/test/Makefile
+++ b/erts/emulator/test/Makefile
@@ -66,6 +66,7 @@ MODULES= \
 	guard_SUITE \
 	hash_SUITE \
 	hibernate_SUITE \
+	hipe_SUITE \
 	list_bif_SUITE \
 	match_spec_SUITE \
 	module_info_SUITE \
diff --git a/erts/emulator/test/hipe_SUITE.erl b/erts/emulator/test/hipe_SUITE.erl
new file mode 100644
index 0000000..66e9c1c
--- /dev/null
+++ b/erts/emulator/test/hipe_SUITE.erl
@@ -0,0 +1,35 @@
+-module(hipe_SUITE).
+
+-include("test_server.hrl").
+
+-export([all/1,call_beam_module_with_on_load/1]).
+
+all(suite) ->
+    [call_beam_module_with_on_load].
+
+call_beam_module_with_on_load(suite) ->
+    [];
+call_beam_module_with_on_load(doc) ->
+    ["Test that we can call a non-native module with a on_load from a native module"];
+call_beam_module_with_on_load(Config) when is_list(Config) ->
+    ?line DataDir = ?config(data_dir, Config),
+    ?line CallerModule = filename:join(DataDir, "native_caller.erl"),
+    ?line CalleeModule = filename:join(DataDir, "beam_callee.erl"),
+    ?line PrivDir = ?config(priv_dir, Config),
+    test_call_with_on_load(PrivDir, CallerModule, CalleeModule).
+
+test_call_with_on_load(PrivDir, CallerModule, CalleeModule) ->
+    % Compile both modules.
+    ?line {ok, CallerModuleName} = compile:file(CallerModule, [{outdir, PrivDir}]),
+    ?line {ok, CalleeModuleName} = compile:file(CalleeModule, [{outdir, PrivDir}]),
+    % Add PrivDir to the path so we can load modules interactively.
+    AbsPrivDir = filename:absname(PrivDir),
+    ?line true = code:add_patha(AbsPrivDir),
+    % Call the function in caller module.
+    ?line ok = CallerModuleName:call(CalleeModuleName),
+    % Purge both modules now.
+    ?line true = code:soft_purge(CallerModuleName),
+    ?line true = code:soft_purge(CalleeModuleName),
+    % Remove PrivDir from the path.
+    ?line true = code:del_path(AbsPrivDir),
+    ok.
diff --git a/erts/emulator/test/hipe_SUITE_data/beam_callee.erl b/erts/emulator/test/hipe_SUITE_data/beam_callee.erl
new file mode 100644
index 0000000..14342fc
--- /dev/null
+++ b/erts/emulator/test/hipe_SUITE_data/beam_callee.erl
@@ -0,0 +1,10 @@
+-module(beam_callee).
+-on_load(on_load/0).
+-export([f/0]).
+
+on_load() ->
+    ok.
+
+f() ->
+    false = code:is_module_native(?MODULE),
+    ok.
diff --git a/erts/emulator/test/hipe_SUITE_data/native_caller.erl b/erts/emulator/test/hipe_SUITE_data/native_caller.erl
new file mode 100644
index 0000000..9176c37
--- /dev/null
+++ b/erts/emulator/test/hipe_SUITE_data/native_caller.erl
@@ -0,0 +1,9 @@
+-module(native_caller).
+-compile([native]).
+-export([call/1]).
+
+call(CalleeModuleName) ->
+    true = code:is_module_native(?MODULE),
+    ok = CalleeModuleName:f(),
+    ok = CalleeModuleName:f(),
+    ok.
-- 
1.7.1.1



More information about the erlang-patches mailing list