From 39072836944d00c288beebfd98b14593f9609006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20L=C3=A5ng?= Date: Thu, 19 May 2016 17:38:54 +0200 Subject: Add a loader state for HiPE code loading Just like the BEAM loader state (as returned by erlang:prepare_loading/2), the HiPE loader state is contained in a magic binary. Eventually, we will separate HiPE loading into a prepare and a finalise phase, like the BEAM loader, where the prepare phase will be implemented by hipe_unified_loader and the finalise phase be implemented in C by hipe_load.c and beam_load.c, making prepare side-effect free and finalise atomic. The finalise phase will be exposed through the erlang:finish_loading/1 API, just like the BEAM loader, as this will allow HiPE and BEAM modules to be mixed in the same atomic "commit". The usage of a loader state makes it easier to keep track of all resources allocated during loading, and will not only make it easy to prevent leaks when hipe_unified_loader crashes, but also paves the way for proper, leak-free, unloading of HiPE modules. --- erts/emulator/hipe/hipe_load.c | 101 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 erts/emulator/hipe/hipe_load.c (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c new file mode 100644 index 0000000000..5bce3f1aee --- /dev/null +++ b/erts/emulator/hipe/hipe_load.c @@ -0,0 +1,101 @@ +/* + * %CopyrightBegin% + + * + * Copyright Ericsson AB 2016. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * %CopyrightEnd% + */ +/* + * hipe_load.c + * + * HiPE atomic code loader + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif +#include "sys.h" +#include "global.h" +#include "erl_binary.h" +#include "hipe_load.h" + +/* + * This destructor function can safely be called multiple times. + */ +static void +hipe_loader_state_dtor(Binary* magic) +{ + HipeLoaderState* stp = ERTS_MAGIC_BIN_DATA(magic); + if (stp->module == NIL) return; + + erts_fprintf(stderr, "Destroying HiPE loader state for module %T\n", + stp->module); + + // TODO: Needs to be freed separately. We'd like have a unified executable + // code allocator, so postpone this for now. + /* if (stp->text_segment) */ + /* erts_free(ERTS_ALC_T_HIPE, stp->text_segment); */ + stp->text_segment = NULL; + stp->text_segment_size = 0; + + if (stp->data_segment) + erts_free(ERTS_ALC_T_HIPE, stp->data_segment); + stp->data_segment = NULL; + stp->data_segment_size = 0; + + stp->module = NIL; +} + +Binary *hipe_alloc_loader_state(Eterm module) +{ + HipeLoaderState *stp; + Binary *magic; + + if (is_not_atom(module)) return NULL; + + erts_fprintf(stderr, "Creating HiPE loader state for module %T\n", module); + + magic = erts_create_magic_binary(sizeof(HipeLoaderState), + hipe_loader_state_dtor); + erts_refc_inc(&magic->refc, 1); + stp = ERTS_MAGIC_BIN_DATA(magic); + + stp->module = module; + stp->text_segment = NULL; + stp->text_segment_size = 0; + stp->data_segment = NULL; + stp->data_segment_size = 0; + + return magic; +} + +void hipe_free_loader_state(Binary *magic) +{ + if (ERTS_MAGIC_BIN_DESTRUCTOR(magic) != hipe_loader_state_dtor) + return; + + /* Why does BEAM do a refc_dec here? What is holding that reference? */ + hipe_loader_state_dtor(magic); +} + +HipeLoaderState * +hipe_get_loader_state(Binary *magic) +{ + if (ERTS_MAGIC_BIN_DESTRUCTOR(magic) != hipe_loader_state_dtor) + return NULL; + + return (HipeLoaderState*) ERTS_MAGIC_BIN_DATA(magic); +} -- cgit v1.2.3 From 966098ceb9dd9d18e9bcd37cd06b96045903e320 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 20 Sep 2016 16:16:50 +0200 Subject: erts: Move new hipe ref and sdesc lists to loader state --- erts/emulator/hipe/hipe_load.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index 5bce3f1aee..cebbbafdd3 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -79,6 +79,9 @@ Binary *hipe_alloc_loader_state(Eterm module) stp->data_segment = NULL; stp->data_segment_size = 0; + stp->new_hipe_refs = NULL; + stp->new_hipe_sdesc = NULL; + return magic; } -- cgit v1.2.3 From e77d7a8417368617c4c228af9556a7f6a8f3e84c Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 28 Sep 2016 20:55:40 +0200 Subject: erts: Fix early hipe patch loading by introducing hipe_bifs:commit_patch_load/1 that creates the HipeModule. --- erts/emulator/hipe/hipe_load.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index cebbbafdd3..48f0cab293 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -57,6 +57,11 @@ hipe_loader_state_dtor(Binary* magic) stp->data_segment_size = 0; stp->module = NIL; + + ASSERT(!stp->new_hipe_refs && !stp->new_hipe_sdesc); + /* ToDO: Purge lists 'new_hipe_refs' and 'new_hipe_sdesc' + * if this is a failed load. + */ } Binary *hipe_alloc_loader_state(Eterm module) -- cgit v1.2.3 From 1e8588e8c310a628fb0215ef15dc3cc29f98c336 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Sun, 2 Oct 2016 16:17:21 +0200 Subject: erts: Refactor hipe_loader_state_dtor into a true destructor that is only called once. Basically switch hipe_free_loader_state and hipe_loader_state_dtor. --- erts/emulator/hipe/hipe_load.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index 48f0cab293..6261857d8f 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -32,13 +32,8 @@ #include "erl_binary.h" #include "hipe_load.h" -/* - * This destructor function can safely be called multiple times. - */ -static void -hipe_loader_state_dtor(Binary* magic) +void hipe_free_loader_state(HipeLoaderState *stp) { - HipeLoaderState* stp = ERTS_MAGIC_BIN_DATA(magic); if (stp->module == NIL) return; erts_fprintf(stderr, "Destroying HiPE loader state for module %T\n", @@ -64,6 +59,16 @@ hipe_loader_state_dtor(Binary* magic) */ } +static void +hipe_loader_state_dtor(Binary* magic) +{ + HipeLoaderState* stp = ERTS_MAGIC_BIN_DATA(magic); + + ASSERT(ERTS_MAGIC_BIN_DESTRUCTOR(magic) == hipe_loader_state_dtor); + + hipe_free_loader_state(stp); +} + Binary *hipe_alloc_loader_state(Eterm module) { HipeLoaderState *stp; @@ -90,15 +95,6 @@ Binary *hipe_alloc_loader_state(Eterm module) return magic; } -void hipe_free_loader_state(Binary *magic) -{ - if (ERTS_MAGIC_BIN_DESTRUCTOR(magic) != hipe_loader_state_dtor) - return; - - /* Why does BEAM do a refc_dec here? What is holding that reference? */ - hipe_loader_state_dtor(magic); -} - HipeLoaderState * hipe_get_loader_state(Binary *magic) { -- cgit v1.2.3 From d9e8fa3d13be54246297ac9776e835ce0bdcd362 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Sun, 2 Oct 2016 22:51:49 +0200 Subject: erts: Free hipe_refs and hipe_sdesc of a failed load --- erts/emulator/hipe/hipe_load.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index 6261857d8f..c05dd886a4 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -31,6 +31,7 @@ #include "global.h" #include "erl_binary.h" #include "hipe_load.h" +#include "hipe_bif0.h" void hipe_free_loader_state(HipeLoaderState *stp) { @@ -51,12 +52,16 @@ void hipe_free_loader_state(HipeLoaderState *stp) stp->data_segment = NULL; stp->data_segment_size = 0; - stp->module = NIL; + if (stp->new_hipe_refs) { + hipe_purge_refs(stp->new_hipe_refs, stp->module); + stp->new_hipe_refs = NULL; + } + if (stp->new_hipe_sdesc) { + hipe_purge_sdescs(stp->new_hipe_sdesc, stp->module); + stp->new_hipe_sdesc = NULL; + } - ASSERT(!stp->new_hipe_refs && !stp->new_hipe_sdesc); - /* ToDO: Purge lists 'new_hipe_refs' and 'new_hipe_sdesc' - * if this is a failed load. - */ + stp->module = NIL; } static void -- cgit v1.2.3 From d347e91735cce9ace9c376ba4913fcf688da22f8 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Mon, 3 Oct 2016 19:22:08 +0200 Subject: erts: Remove debug printout for hipe loader state creation and destruction. --- erts/emulator/hipe/hipe_load.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index c05dd886a4..fcbcf1a6a4 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -37,9 +37,6 @@ void hipe_free_loader_state(HipeLoaderState *stp) { if (stp->module == NIL) return; - erts_fprintf(stderr, "Destroying HiPE loader state for module %T\n", - stp->module); - // TODO: Needs to be freed separately. We'd like have a unified executable // code allocator, so postpone this for now. /* if (stp->text_segment) */ @@ -81,8 +78,6 @@ Binary *hipe_alloc_loader_state(Eterm module) if (is_not_atom(module)) return NULL; - erts_fprintf(stderr, "Creating HiPE loader state for module %T\n", module); - magic = erts_create_magic_binary(sizeof(HipeLoaderState), hipe_loader_state_dtor); erts_refc_inc(&magic->refc, 1); -- cgit v1.2.3 From 3582e6f420d84ddac64b55cb13100f1ae7f31b08 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Thu, 13 Oct 2016 20:59:58 +0200 Subject: erts: Replace unsafe Module.first_hipe_ref with hash table mod2mfa_tab --- erts/emulator/hipe/hipe_load.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'erts/emulator/hipe/hipe_load.c') diff --git a/erts/emulator/hipe/hipe_load.c b/erts/emulator/hipe/hipe_load.c index fcbcf1a6a4..2998ed87a2 100644 --- a/erts/emulator/hipe/hipe_load.c +++ b/erts/emulator/hipe/hipe_load.c @@ -50,11 +50,11 @@ void hipe_free_loader_state(HipeLoaderState *stp) stp->data_segment_size = 0; if (stp->new_hipe_refs) { - hipe_purge_refs(stp->new_hipe_refs, stp->module); + hipe_purge_refs(stp->new_hipe_refs, stp->module, 0); stp->new_hipe_refs = NULL; } if (stp->new_hipe_sdesc) { - hipe_purge_sdescs(stp->new_hipe_sdesc, stp->module); + hipe_purge_sdescs(stp->new_hipe_sdesc, stp->module, 0); stp->new_hipe_sdesc = NULL; } -- cgit v1.2.3