From b07e9f5652106a4b07335b51763192421b1671c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 22 Nov 2011 15:34:24 +0100 Subject: beam_load.c: apply/2 does not need a special case It is wrongly assumed in the BEAM loader that apply/2 is a BIF and must be treated specially. Also make it clearer in ops.tab that apply/3 is a BIF, but apply/2 is not. --- erts/emulator/beam/beam_load.c | 25 ++++--------------------- erts/emulator/beam/ops.tab | 12 ++++++++---- 2 files changed, 12 insertions(+), 25 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index dd788df6e4..a510632220 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -1315,15 +1315,6 @@ load_import_table(LoaderState* stp) static int read_export_table(LoaderState* stp) { - static struct { - Eterm mod; - Eterm func; - int arity; - } allow_redef[] = { - /* The BIFs that are allowed to be redefined by Erlang code */ - {am_erlang,am_apply,2}, - {am_erlang,am_apply,3}, - }; int i; GetInt(stp, 4, stp->num_exps); @@ -1361,21 +1352,13 @@ read_export_table(LoaderState* stp) stp->export[i].address = stp->code + value; /* - * Check that we are not redefining a BIF (except the ones allowed to - * redefine). + * Check that we are not redefining a BIF (except erlang:apply/3). */ if ((e = erts_find_export_entry(stp->module, func, arity)) != NULL) { if (e->code[3] == (BeamInstr) em_apply_bif) { - int j; - - for (j = 0; j < sizeof(allow_redef)/sizeof(allow_redef[0]); j++) { - if (stp->module == allow_redef[j].mod && - func == allow_redef[j].func && - arity == allow_redef[j].arity) { - break; - } - } - if (j == sizeof(allow_redef)/sizeof(allow_redef[0])) { + if (stp->module != am_erlang || + func != am_apply || + arity != 3) { LoadError2(stp, "exported function %T/%d redefines BIF", func, arity); } diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab index fc53a88a3a..f051f6aa5b 100644 --- a/erts/emulator/beam/ops.tab +++ b/erts/emulator/beam/ops.tab @@ -829,16 +829,20 @@ call_ext_only Ar=u==2 Bif=u$bif:erlang:load_nif/2 => allocate u Ar | i_call_ext # -# The apply/2 and apply/3 BIFs are instructions. +# apply/2 is an instruction, not a BIF. # call_ext u==2 u$func:erlang:apply/2 => i_apply_fun call_ext_last u==2 u$func:erlang:apply/2 D => i_apply_fun_last D call_ext_only u==2 u$func:erlang:apply/2 => i_apply_fun_only -call_ext u==3 u$func:erlang:apply/3 => i_apply -call_ext_last u==3 u$func:erlang:apply/3 D => i_apply_last D -call_ext_only u==3 u$func:erlang:apply/3 => i_apply_only +# +# The apply/3 BIF is an instruction. +# + +call_ext u==3 u$bif:erlang:apply/3 => i_apply +call_ext_last u==3 u$bif:erlang:apply/3 D => i_apply_last D +call_ext_only u==3 u$bif:erlang:apply/3 => i_apply_only # # The exit/1 and throw/1 BIFs never execute the instruction following them; -- cgit v1.2.3 From 7b8b454ee8c94abe789377590423506b95a5e109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 22 Nov 2011 13:19:44 +0100 Subject: beam_load.c: Remove useless call to next_heap_size() The idea was probably to cause less fragmentation. Even if that would be true, it is irrelevant because the short-lived allocator that is used does not have any problems with fragmentation. In CodeNeed() we will simply double the size of the area used for code instead of using the next heap size. --- erts/emulator/beam/beam_load.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index a510632220..3baef6906e 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -666,7 +666,7 @@ erts_prepare_loading(LoaderState* stp, Process *c_p, Eterm group_leader, /* * Initialize code area. */ - stp->code_buffer_size = erts_next_heap_size(2048 + stp->num_functions, 0); + stp->code_buffer_size = 2048 + stp->num_functions; stp->code = (BeamInstr *) erts_alloc(ERTS_ALC_T_CODE, sizeof(BeamInstr) * stp->code_buffer_size); @@ -1218,8 +1218,7 @@ load_atom_table(LoaderState* stp) GetInt(stp, 4, stp->num_atoms); stp->num_atoms++; stp->atom = erts_alloc(ERTS_ALC_T_LOADER_TMP, - erts_next_heap_size((stp->num_atoms*sizeof(Eterm)), - 0)); + stp->num_atoms*sizeof(Eterm)); /* * Read all atoms. @@ -1264,9 +1263,7 @@ load_import_table(LoaderState* stp) GetInt(stp, 4, stp->num_imports); stp->import = erts_alloc(ERTS_ALC_T_LOADER_TMP, - erts_next_heap_size((stp->num_imports * - sizeof(ImportEntry)), - 0)); + stp->num_imports * sizeof(ImportEntry)); for (i = 0; i < stp->num_imports; i++) { int n; Eterm mod; @@ -1686,9 +1683,9 @@ read_code_header(LoaderState* stp) #define CodeNeed(w) do { \ ASSERT(ci <= code_buffer_size); \ if (code_buffer_size < ci+(w)) { \ - code_buffer_size = erts_next_heap_size(ci+(w), 0); \ - stp->code = code \ - = (BeamInstr *) erts_realloc(ERTS_ALC_T_CODE, \ + code_buffer_size = 2*ci+(w); \ + stp->code = code = \ + (BeamInstr *) erts_realloc(ERTS_ALC_T_CODE, \ (void *) code, \ code_buffer_size * sizeof(BeamInstr)); \ } \ -- cgit v1.2.3 From e71de38e077199d7d36cf5e3e6017cdd12dff1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 10 Jan 2012 07:56:32 +0100 Subject: beam_load.c: Don't show unnecessary context in errors For errors that occur after reading the code chunking, saying that the error occurred in the last function in the module and in the instruction int_code_end/0 is just confusing. --- erts/emulator/beam/beam_load.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index 3baef6906e..d9f3e3f4c6 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -2437,6 +2437,9 @@ load_code(LoaderState* stp) case op_int_code_end: stp->code_buffer_size = code_buffer_size; stp->ci = ci; + stp->function = THE_NON_VALUE; + stp->genop = NULL; + stp->specific_op = -1; return 1; } -- cgit v1.2.3 From 64ccd8c9b7a782ca777ca4649dbb1f4a1ef00bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 10 Jan 2012 08:31:15 +0100 Subject: beam_load.c: Allow stubs for BIFs We want to be able to write type specifications for BIFs in the same way as for any other function. Currently, the type for BIFs need to be described in erl_bif_types. To avoid extending the compiler and Dialyzer with special directives for providing specifications for BIFs, we have decided to let the loader accept a local definition for a function which exists as a BIF. As an example, here is how a stub for lists:reverse/2 can be defined: -export([reverse/2]). -spec reverse([term()], term()) -> [term()]. reverse(_, _) -> erlang:nif_error(undef). Essentially, the loader will discard the local definition of reverse/2. Other functions in the same module must *not* do local calls to a BIF stub. If a local call to a BIF is found, the loader will refuse to load the module. That is, the following call is not allowed: reverse(List) -> reverse(List, []). but the following is: reverse(List) -> ?MODULE:reverse(List, []). A few words about the implementation. It turns out to be too complicated to actually discard the BIF stubs. Although it would be possibly with some jiggery pokery in ops.tab, the code would be difficult to maintain and it could slow down loading of modules that don't define BIFs (which are almost all modules). Therefore, the stub functions are kept in the loaded code, but their names in the func_info instruction are invalidated so that module_info(functions) can filter them out. --- erts/emulator/beam/beam_load.c | 108 ++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 23 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index d9f3e3f4c6..1f9635a6c1 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -507,6 +507,7 @@ static int verify_chunks(LoaderState* stp); static int load_atom_table(LoaderState* stp); static int load_import_table(LoaderState* stp); static int read_export_table(LoaderState* stp); +static int is_bif(Eterm mod, Eterm func, unsigned arity); static int read_lambda_table(LoaderState* stp); static int read_literal_table(LoaderState* stp); static int read_line_table(LoaderState* stp); @@ -1313,6 +1314,7 @@ static int read_export_table(LoaderState* stp) { int i; + BeamInstr* address; GetInt(stp, 4, stp->num_exps); if (stp->num_exps > stp->num_functions) { @@ -1328,7 +1330,6 @@ read_export_table(LoaderState* stp) Uint value; Eterm func; Uint arity; - Export* e; GetInt(stp, 4, n); GetAtom(stp, n, func); @@ -1346,21 +1347,34 @@ read_export_table(LoaderState* stp) if (value == 0) { LoadError2(stp, "export table entry %d: label %d not resolved", i, n); } - stp->export[i].address = stp->code + value; + stp->export[i].address = address = stp->code + value; /* - * Check that we are not redefining a BIF (except erlang:apply/3). + * Find out if there is a BIF with the same name. */ - if ((e = erts_find_export_entry(stp->module, func, arity)) != NULL) { - if (e->code[3] == (BeamInstr) em_apply_bif) { - if (stp->module != am_erlang || - func != am_apply || - arity != 3) { - LoadError2(stp, "exported function %T/%d redefines BIF", - func, arity); - } - } + + if (!is_bif(stp->module, func, arity)) { + continue; } + + /* + * This is a stub for a BIF. + * + * It should not be exported, and the information in its + * func_info instruction should be invalidated so that it + * can be filtered out by module_info(functions) and by + * any other functions that walk through all local functions. + */ + + if (stp->labels[n].patches) { + LoadError3(stp, "there are local calls to the stub for " + "the BIF %T:%T/%d", + stp->module, func, arity); + } + stp->export[i].address = NULL; + address[-1] = 0; + address[-2] = NIL; + address[-3] = NIL; } return 1; @@ -1368,6 +1382,27 @@ read_export_table(LoaderState* stp) return 0; } + +static int +is_bif(Eterm mod, Eterm func, unsigned arity) +{ + Export* e = erts_find_export_entry(mod, func, arity); + if (e == NULL) { + return 0; + } + if (e->code[3] != (BeamInstr) em_apply_bif) { + return 0; + } + if (mod == am_erlang && func == am_apply && arity == 3) { + /* + * erlang:apply/3 is a special case -- it is implemented + * as an instruction and it is OK to redefine it. + */ + return 0; + } + return 1; +} + static int read_lambda_table(LoaderState* stp) { @@ -4255,17 +4290,24 @@ final_touch(LoaderState* stp) */ for (i = 0; i < stp->num_exps; i++) { - Export* ep = erts_export_put(stp->module, stp->export[i].function, - stp->export[i].arity); + Export* ep; + BeamInstr* address = stp->export[i].address; + + if (address == NULL) { + /* Skip stub for a BIF */ + continue; + } + ep = erts_export_put(stp->module, stp->export[i].function, + stp->export[i].arity); if (!on_load) { - ep->address = stp->export[i].address; + ep->address = address; } else { /* * Don't make any of the exported functions * callable yet. */ ep->address = ep->code+3; - ep->code[4] = (BeamInstr) stp->export[i].address; + ep->code[4] = (BeamInstr) address; } } @@ -5037,7 +5079,9 @@ functions_in_module(Process* p, /* Process whose heap to use. */ BeamInstr* code; int i; Uint num_functions; + Uint need; Eterm* hp; + Eterm* hp_end; Eterm result = NIL; if (is_not_atom(mod)) { @@ -5050,19 +5094,28 @@ functions_in_module(Process* p, /* Process whose heap to use. */ } code = modp->code; num_functions = code[MI_NUM_FUNCTIONS]; - hp = HAlloc(p, 5*num_functions); + need = 5*num_functions; + hp = HAlloc(p, need); + hp_end = hp + need; for (i = num_functions-1; i >= 0 ; i--) { BeamInstr* func_info = (BeamInstr *) code[MI_FUNCTIONS+i]; Eterm name = (Eterm) func_info[3]; int arity = (int) func_info[4]; Eterm tuple; - ASSERT(is_atom(name)); - tuple = TUPLE2(hp, name, make_small(arity)); - hp += 3; - result = CONS(hp, tuple, result); - hp += 2; + /* + * If the function name is [], this entry is a stub for + * a BIF that should be ignored. + */ + ASSERT(is_atom(name) || is_nil(name)); + if (is_atom(name)) { + tuple = TUPLE2(hp, name, make_small(arity)); + hp += 3; + result = CONS(hp, tuple, result); + hp += 2; + } } + HRelease(p, hp_end, hp); return result; } @@ -5610,19 +5663,28 @@ stub_final_touch(LoaderState* stp, BeamInstr* fp) { int i; int n = stp->num_exps; + Eterm mod = fp[2]; Eterm function = fp[3]; int arity = fp[4]; #ifdef HIPE Lambda* lp; #endif + if (is_bif(mod, function, arity)) { + fp[1] = 0; + fp[2] = 0; + fp[3] = 0; + fp[4] = 0; + return; + } + /* * Test if the function should be exported. */ for (i = 0; i < n; i++) { if (stp->export[i].function == function && stp->export[i].arity == arity) { - Export* ep = erts_export_put(fp[2], function, arity); + Export* ep = erts_export_put(mod, function, arity); ep->address = fp+5; return; } -- cgit v1.2.3