From f58577787c58fe64f45721eea96b65fefec03261 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 22 Oct 2024 14:20:08 +0200 Subject: [PATCH 1/3] erts: Avoid SEGV in crash dump of global literals on debug target by making the chunk writable before we adjust the 'end' of the area for correct crash dumping. --- erts/emulator/beam/erl_global_literals.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/erts/emulator/beam/erl_global_literals.c b/erts/emulator/beam/erl_global_literals.c index cd1a1109a6f..b9ff36dbe60 100644 --- a/erts/emulator/beam/erl_global_literals.c +++ b/erts/emulator/beam/erl_global_literals.c @@ -81,6 +81,12 @@ ErtsLiteralArea *erts_global_literal_iterate_area(ErtsLiteralArea *prev) next = global_literal_chunk; } +#ifdef DEBUG + erts_mem_guard(next, + (byte*)next->area.end - (byte*)next, + 1, + 1); +#endif next->area.end = next->hp; return &next->area; } From adb4d3ab92aa44c7904cf5215fd374e431143707 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 22 Oct 2024 15:31:39 +0200 Subject: [PATCH 2/3] erts: Refactor global_literal_chunk Change meaning of chunk->area.end from "capacity of the area" to mean the size of the area currently filled with literals. By doing that we don't have to touch it when crash dumping. Instead introduce chunk->chunk_end to denote chunk capacity. --- erts/emulator/beam/atom.c | 2 +- erts/emulator/beam/beam_file.c | 2 +- erts/emulator/beam/dist.c | 2 +- erts/emulator/beam/erl_bif_info.c | 4 +- erts/emulator/beam/erl_global_literals.c | 69 +++++++++++++----------- erts/emulator/beam/erl_global_literals.h | 2 +- erts/emulator/beam/export.c | 2 +- 7 files changed, 45 insertions(+), 38 deletions(-) diff --git a/erts/emulator/beam/atom.c b/erts/emulator/beam/atom.c index 390ca044356..809caa40ff9 100644 --- a/erts/emulator/beam/atom.c +++ b/erts/emulator/beam/atom.c @@ -152,7 +152,7 @@ atom_alloc(Atom* tmpl) 0, tmpl->len, tmpl->u.name); - erts_global_literal_register(&obj->u.bin, hp, heap_size); + erts_global_literal_register(&obj->u.bin); } obj->len = tmpl->len; diff --git a/erts/emulator/beam/beam_file.c b/erts/emulator/beam/beam_file.c index 03f488ceb8e..df05aa32246 100644 --- a/erts/emulator/beam/beam_file.c +++ b/erts/emulator/beam/beam_file.c @@ -1331,7 +1331,7 @@ void beamfile_init(void) { hp = erts_global_literal_allocate(8, &ohp); suffix = erts_bin_bytes_to_list(NIL, hp, (byte*)".erl", 4, 0); - erts_global_literal_register(&suffix, hp, 8); + erts_global_literal_register(&suffix); ERTS_GLOBAL_LIT_ERL_FILE_SUFFIX = suffix; } diff --git a/erts/emulator/beam/dist.c b/erts/emulator/beam/dist.c index 74d3b6a76b7..906ea8de593 100644 --- a/erts/emulator/beam/dist.c +++ b/erts/emulator/beam/dist.c @@ -1163,7 +1163,7 @@ void init_dist(void) if (hpp) { ASSERT(is_value(tuple)); ASSERT(hp == hp_start + sz); - erts_global_literal_register(&tuple, hp, sz); + erts_global_literal_register(&tuple); ERTS_GLOBAL_LIT_DFLAGS_RECORD = tuple; break; } diff --git a/erts/emulator/beam/erl_bif_info.c b/erts/emulator/beam/erl_bif_info.c index d3c01e1bb50..9f0cadd825e 100644 --- a/erts/emulator/beam/erl_bif_info.c +++ b/erts/emulator/beam/erl_bif_info.c @@ -6273,7 +6273,7 @@ static void os_info_init(void) hp = erts_global_literal_allocate(3, &ohp); tuple = TUPLE2(hp, type, flav); - erts_global_literal_register(&tuple, hp, 3); + erts_global_literal_register(&tuple); ERTS_GLOBAL_LIT_OS_TYPE = tuple; hp = erts_global_literal_allocate(4, &ohp); @@ -6282,7 +6282,7 @@ static void os_info_init(void) make_small(major), make_small(minor), make_small(build)); - erts_global_literal_register(&tuple, hp, 4); + erts_global_literal_register(&tuple); ERTS_GLOBAL_LIT_OS_VERSION = tuple; } diff --git a/erts/emulator/beam/erl_global_literals.c b/erts/emulator/beam/erl_global_literals.c index b9ff36dbe60..472ba3ee32d 100644 --- a/erts/emulator/beam/erl_global_literals.c +++ b/erts/emulator/beam/erl_global_literals.c @@ -55,11 +55,13 @@ erts_mtx_t global_literal_lock; * This is protected by the global literal lock. */ struct global_literal_chunk { struct global_literal_chunk *next; - Eterm *hp; + Eterm *chunk_end; ErtsLiteralArea area; } *global_literal_chunk = NULL; +/* The size of the global literal term that is being built */ +Uint global_literal_build_size; ErtsLiteralArea *erts_global_literal_iterate_area(ErtsLiteralArea *prev) @@ -81,13 +83,6 @@ ErtsLiteralArea *erts_global_literal_iterate_area(ErtsLiteralArea *prev) next = global_literal_chunk; } -#ifdef DEBUG - erts_mem_guard(next, - (byte*)next->area.end - (byte*)next, - 1, - 1); -#endif - next->area.end = next->hp; return &next->area; } @@ -106,10 +101,14 @@ static void expand_shared_global_literal_area(Uint heap_size) address = (UWord) erts_alloc(ERTS_ALC_T_LITERAL, size + sys_page_size * 2); address = (address + (sys_page_size - 1)) & ~(sys_page_size - 1); chunk = (struct global_literal_chunk *) address; + + for (int i = 0; i < heap_size; i++) { + chunk->area.start[i] = ERTS_HOLE_MARKER; + } #endif - chunk->hp = &chunk->area.start[0]; - chunk->area.end = &chunk->hp[heap_size]; + chunk->area.end = &(chunk->area.start[0]); + chunk->chunk_end = &(chunk->area.start[heap_size]); chunk->area.off_heap = NULL; chunk->next = global_literal_chunk; @@ -118,45 +117,53 @@ static void expand_shared_global_literal_area(Uint heap_size) Eterm *erts_global_literal_allocate(Uint heap_size, struct erl_off_heap_header ***ohp) { - Eterm *hp; - erts_mtx_lock(&global_literal_lock); - ASSERT((global_literal_chunk->hp <= global_literal_chunk->area.end && - global_literal_chunk->hp >= global_literal_chunk->area.start) ); - if (global_literal_chunk->area.end - global_literal_chunk->hp <= heap_size) { + ASSERT(global_literal_chunk->area.end <= global_literal_chunk->chunk_end && + global_literal_chunk->area.end >= global_literal_chunk->area.start); + if (global_literal_chunk->chunk_end - global_literal_chunk->area.end < heap_size) { expand_shared_global_literal_area(heap_size + GLOBAL_LITERAL_EXPAND_SIZE); } + *ohp = &global_literal_chunk->area.off_heap; + #ifdef DEBUG { struct global_literal_chunk *chunk = global_literal_chunk; erts_mem_guard(chunk, - (chunk->area.end - (Eterm*)chunk) * sizeof(Eterm), + (byte*)(chunk->area.end + heap_size) - (byte*)chunk, 1, 1); } #endif + global_literal_build_size = heap_size; - *ohp = &global_literal_chunk->area.off_heap; - hp = global_literal_chunk->hp; - global_literal_chunk->hp += heap_size; - - return hp; + return global_literal_chunk->area.end; } -void erts_global_literal_register(Eterm *variable, Eterm *hp, Uint heap_size) { - erts_set_literal_tag(variable, hp, heap_size); +void erts_global_literal_register(Eterm *variable) { + struct global_literal_chunk *chunk = global_literal_chunk; + + ASSERT(ptr_val(*variable) >= chunk->area.end && + ptr_val(*variable) < (chunk->area.end + global_literal_build_size)); + + erts_set_literal_tag(variable, + chunk->area.end, + global_literal_build_size); + chunk->area.end += global_literal_build_size; + + ASSERT(chunk->area.end <= chunk->chunk_end && + chunk->area.end >= chunk->area.start); + ASSERT(chunk->area.end == chunk->chunk_end || + chunk->area.end[global_literal_build_size] == ERTS_HOLE_MARKER); + #ifdef DEBUG - { - struct global_literal_chunk *chunk = global_literal_chunk; - erts_mem_guard(chunk, - (chunk->area.end - (Eterm*)chunk) * sizeof(Eterm), - 1, - 0); - } + erts_mem_guard(chunk, + (byte*)chunk->chunk_end - (byte*)chunk, + 1, + 0); #endif erts_mtx_unlock(&global_literal_lock); @@ -169,7 +176,7 @@ static void init_empty_tuple(void) { hp[0] = make_arityval_zero(); hp[1] = make_arityval_zero(); tuple = make_tuple(hp); - erts_global_literal_register(&tuple, hp, 2); + erts_global_literal_register(&tuple); ERTS_GLOBAL_LIT_EMPTY_TUPLE = tuple; } diff --git a/erts/emulator/beam/erl_global_literals.h b/erts/emulator/beam/erl_global_literals.h index fbf80a612e0..bc8b4472011 100644 --- a/erts/emulator/beam/erl_global_literals.h +++ b/erts/emulator/beam/erl_global_literals.h @@ -47,7 +47,7 @@ Eterm *erts_global_literal_allocate(Uint sz, struct erl_off_heap_header ***ohp); /* Registers the pointed-to term as a global literal. Must be called for terms * allocated using erts_global_literal_allocate.*/ -void erts_global_literal_register(Eterm *variable, Eterm *hp, Uint heap_size); +void erts_global_literal_register(Eterm *variable); /* Iterates between global literal areas. Can only be used when crash dumping. * Iteration is started by passing NULL, then successively calling this function diff --git a/erts/emulator/beam/export.c b/erts/emulator/beam/export.c index a471c561ecd..edccaaa2a86 100644 --- a/erts/emulator/beam/export.c +++ b/erts/emulator/beam/export.c @@ -51,7 +51,7 @@ static void create_shared_lambda(Export *export) export->lambda = make_fun(lambda); - erts_global_literal_register(&export->lambda, (Eterm*)lambda, ERL_FUN_SIZE); + erts_global_literal_register(&export->lambda); } static HashValue export_hash(const Export *export) From 45c1a843399bde2b3cce112d9db18af624148727 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 22 Oct 2024 18:50:50 +0200 Subject: [PATCH 3/3] fixup! erts: Refactor global_literal_chunk --- erts/emulator/beam/erl_global_literals.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erts/emulator/beam/erl_global_literals.c b/erts/emulator/beam/erl_global_literals.c index 472ba3ee32d..0db33920870 100644 --- a/erts/emulator/beam/erl_global_literals.c +++ b/erts/emulator/beam/erl_global_literals.c @@ -157,7 +157,7 @@ void erts_global_literal_register(Eterm *variable) { ASSERT(chunk->area.end <= chunk->chunk_end && chunk->area.end >= chunk->area.start); ASSERT(chunk->area.end == chunk->chunk_end || - chunk->area.end[global_literal_build_size] == ERTS_HOLE_MARKER); + chunk->area.end[0] == ERTS_HOLE_MARKER); #ifdef DEBUG erts_mem_guard(chunk,