Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix hang when calling functions in a module with an on_load attribute
from a native module

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.
  • Loading branch information
pguyot committed Sep 22, 2010
1 parent dd51118 commit 442599d
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 4 deletions.
9 changes: 8 additions & 1 deletion erts/emulator/beam/beam_bif_load.c
Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions erts/emulator/beam/beam_emu.c
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -5034,6 +5051,7 @@ void process_main(void)
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);
Expand Down
1 change: 1 addition & 0 deletions erts/emulator/beam/beam_load.h
Expand Up @@ -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. */
Expand Down
7 changes: 4 additions & 3 deletions erts/emulator/beam/export.h
Expand Up @@ -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];
Expand Down
1 change: 1 addition & 0 deletions erts/emulator/test/Makefile
Expand Up @@ -66,6 +66,7 @@ MODULES= \
guard_SUITE \
hash_SUITE \
hibernate_SUITE \
hipe_SUITE \
list_bif_SUITE \
match_spec_SUITE \
module_info_SUITE \
Expand Down
35 changes: 35 additions & 0 deletions 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.
10 changes: 10 additions & 0 deletions 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.
9 changes: 9 additions & 0 deletions 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.

0 comments on commit 442599d

Please sign in to comment.