[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