From 14d589a5abf91a3f27fe602b0a4bdd2cec33e9df Mon Sep 17 00:00:00 2001 From: Paul Guyot Date: Wed, 23 Aug 2023 23:05:47 +0200 Subject: [PATCH] Fix allocation with bs_init2 and bs_init_bits (fix #730) This is a proper fix of #730 and replaces #765 Signed-off-by: Paul Guyot --- src/libAtomVM/opcodesswitch.h | 50 +++++++++++-------- tests/erlang_tests/CMakeLists.txt | 2 + .../test_bs_init2_heap_allocation.erl | 45 +++++++++++++++++ tests/test.c | 2 + 4 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 tests/erlang_tests/test_bs_init2_heap_allocation.erl diff --git a/src/libAtomVM/opcodesswitch.h b/src/libAtomVM/opcodesswitch.h index 6707320df..cb251044d 100644 --- a/src/libAtomVM/opcodesswitch.h +++ b/src/libAtomVM/opcodesswitch.h @@ -1515,7 +1515,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) #endif while (1) { - TRACE("-- loop -- i = %d\n", i); + TRACE("-- loop -- i = %d\n", (int) i); switch (code[i]) { case OP_LABEL: { @@ -1527,7 +1527,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) USED_BY_TRACE(label); #ifdef IMPL_CODE_LOADER - TRACE("Mark label %i here at %i\n", label, i); + TRACE("Mark label %i here at %i\n", label, (int) i); module_add_label(mod, label, &code[i]); #endif @@ -3246,14 +3246,14 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) term *list_elem = term_list_alloc(&ctx->heap); #endif - TRACE("put_list/3\n"); - #ifdef IMPL_CODE_LOADER + TRACE("put_list/3\n"); UNUSED(head); UNUSED(tail); #endif #ifdef IMPL_EXECUTE_LOOP + TRACE("put_list/3 dreg=%c%i\n", T_DEST_REG(dreg_type, dreg)); term t = term_list_init_prepend(list_elem, head, tail); WRITE_REGISTER(dreg_type, dreg, t); #endif @@ -3303,7 +3303,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) case OP_BADMATCH: { #ifdef IMPL_EXECUTE_LOOP - if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + // We can gc as we are raising + if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } #endif @@ -3321,7 +3322,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("badmatch/1, v=0x%lx\n", arg1); term new_error_tuple = term_alloc_tuple(2, &ctx->heap); - //TODO: check alloc term_put_tuple_element(new_error_tuple, 0, BADMATCH_ATOM); term_put_tuple_element(new_error_tuple, 1, arg1); @@ -3354,7 +3354,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) case OP_CASE_END: { #ifdef IMPL_EXECUTE_LOOP - if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + // We can gc as we are raising + if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } #endif @@ -3372,7 +3373,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("case_end/1, v=0x%lx\n", arg1); term new_error_tuple = term_alloc_tuple(2, &ctx->heap); - //TODO: reserve memory before term_put_tuple_element(new_error_tuple, 0, CASE_CLAUSE_ATOM); term_put_tuple_element(new_error_tuple, 1, arg1); @@ -3402,6 +3402,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) term fun = ctx->x[args_count]; if (UNLIKELY(!term_is_function(fun))) { + // We can gc as we are raising if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -3612,7 +3613,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) case OP_TRY_CASE_END: { #ifdef IMPL_EXECUTE_LOOP - if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + // We can gc as we are raising + if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } #endif @@ -3630,7 +3632,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("try_case_end/1, val=%lx\n", arg1); term new_error_tuple = term_alloc_tuple(2, &ctx->heap); - //TODO: ensure memory before term_put_tuple_element(new_error_tuple, 0, TRY_CLAUSE_ATOM); term_put_tuple_element(new_error_tuple, 1, arg1); @@ -3705,8 +3706,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) case ERROR_ATOM_INDEX: { ctx->x[2] = stacktrace_build(ctx, &ctx->x[2]); - - if (UNLIKELY(memory_ensure_free_opt(ctx, 6, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + // MEMORY_CAN_SHRINK because catch_end is classified as gc in beam_ssa_codegen.erl + if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2) * 2, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } term reason_tuple = term_alloc_tuple(2, &ctx->heap); @@ -3720,7 +3721,8 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) break; } case LOWERCASE_EXIT_ATOM_INDEX: { - if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + // MEMORY_CAN_SHRINK because catch_end is classified as gc in beam_ssa_codegen.erl + if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } term exit_tuple = term_alloc_tuple(2, &ctx->heap); @@ -3775,7 +3777,6 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) term size; DECODE_COMPACT_TERM(size, code, i, next_off) uint32_t words; - UNUSED(words); DECODE_LITERAL(words, code, i, next_off) uint32_t regs; UNUSED(regs); @@ -3797,7 +3798,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("bs_init2/6, fail=%u size=%li words=%u regs=%u dreg=%c%i\n", (unsigned) fail, size_val, (unsigned) words, (unsigned) regs, T_DEST_REG(dreg_type, dreg)); - if (UNLIKELY(memory_ensure_free_opt(ctx, term_binary_heap_size(size_val), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + if (UNLIKELY(memory_ensure_free_opt(ctx, words + term_binary_heap_size(size_val), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } term t = term_create_empty_binary(size_val, &ctx->heap, ctx->global); @@ -3846,7 +3847,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("bs_init_bits/6, fail=%i size=%li words=%i regs=%i dreg=%c%i\n", fail, size_val, words, regs, T_DEST_REG(dreg_type, dreg)); - if (UNLIKELY(memory_ensure_free_opt(ctx, term_binary_heap_size(size_val / 8), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { + if (UNLIKELY(memory_ensure_free_opt(ctx, words + term_binary_heap_size(size_val / 8), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } term t = term_create_empty_binary(size_val / 8, &ctx->heap, ctx->global); @@ -4528,7 +4529,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) #ifdef IMPL_EXECUTE_LOOP if (UNLIKELY(!term_is_binary(ctx->bs))) { - TRACE("bs_put_binary: Bad state. ctx->bs is not a binary.\n"); + TRACE("bs_put_string: Bad state. ctx->bs is not a binary.\n"); RAISE_ERROR(BADARG_ATOM); } if (ctx->bs_offset % 8 != 0) { @@ -4536,10 +4537,12 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) RAISE_ERROR(UNSUPPORTED_ATOM); } + TRACE("bs_put_string/2, size=%u offset=%u\n", size, offset); + size_t remaining = 0; const uint8_t *str = module_get_str(mod, offset, &remaining); if (UNLIKELY(IS_NULL_PTR(str))) { - TRACE("bs_put_binary: Bad offset in strings table.\n"); + TRACE("bs_put_string: Bad offset in strings table.\n"); RAISE_ERROR(BADARG_ATOM); } @@ -4570,6 +4573,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) #ifdef IMPL_EXECUTE_LOOP int slots = term_to_int(slots_term); + // MEMORY_CAN_SHRINK because bs_start_match is classified as gc in beam_ssa_codegen.erl if (memory_ensure_free_opt(ctx, TERM_BOXED_BIN_MATCH_STATE_SIZE + slots, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -4601,6 +4605,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) } case OP_BS_START_MATCH3: { + // MEMORY_CAN_SHRINK because bs_start_match is classified as gc in beam_ssa_codegen.erl #ifdef IMPL_EXECUTE_LOOP if (memory_ensure_free_opt(ctx, TERM_BOXED_BIN_MATCH_STATE_SIZE, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); @@ -5637,6 +5642,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) size_t new_map_size = src_size + new_entries; bool is_shared = new_entries == 0; size_t heap_needed = term_map_size_in_terms_maybe_shared(new_map_size, is_shared); + // MEMORY_CAN_SHRINK because put_map is classified as gc in beam_ssa_codegen.erl if (memory_ensure_free_opt(ctx, heap_needed, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -5765,6 +5771,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) // Maybe GC, and reset the src term in case it changed // size_t src_size = term_get_map_size(src); + // MEMORY_CAN_SHRINK because put_map is classified as gc in beam_ssa_codegen.erl if (memory_ensure_free_opt(ctx, term_map_size_in_terms_maybe_shared(src_size, true), MEMORY_CAN_SHRINK) != MEMORY_GC_OK) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -6336,6 +6343,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) case OP_BS_START_MATCH4: { #ifdef IMPL_EXECUTE_LOOP + // MEMORY_CAN_SHRINK because bs_start_match is classified as gc in beam_ssa_codegen.erl if (memory_ensure_free_opt(ctx, TERM_BOXED_BIN_MATCH_STATE_SIZE, MEMORY_CAN_SHRINK) != MEMORY_GC_OK) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -6651,7 +6659,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) VERIFY_IS_INTEGER(size, "bs_create_bin/6"); avm_int_t signed_size_value = term_to_int(size); if (UNLIKELY(signed_size_value < 0)) { - TRACE("bs_create_bin/6: size value less than 0: %i\n", signed_size_value); + TRACE("bs_create_bin/6: size value less than 0: %i\n", (int) signed_size_value); RAISE_ERROR(BADARG_ATOM); } size_value = (size_t) signed_size_value; @@ -6723,7 +6731,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) VERIFY_IS_INTEGER(size, "bs_create_bin/6"); avm_int_t signed_size_value = term_to_int(size); if (UNLIKELY(signed_size_value < 0)) { - TRACE("bs_create_bin/6: size value less than 0: %i\n", signed_size_value); + TRACE("bs_create_bin/6: size value less than 0: %i\n", (int) signed_size_value); RAISE_ERROR(BADARG_ATOM); } size_value = (size_t) signed_size_value; @@ -6767,6 +6775,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) #ifdef IMPL_EXECUTE_LOOP if (UNLIKELY(!term_is_function(fun))) { + // We can gc as we are raising if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -6792,6 +6801,7 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) TRACE("badrecord/1\n"); #ifdef IMPL_EXECUTE_LOOP + // We can gc as we are raising if (UNLIKELY(memory_ensure_free_opt(ctx, TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } diff --git a/tests/erlang_tests/CMakeLists.txt b/tests/erlang_tests/CMakeLists.txt index cbaff4d7a..77c79b6cb 100644 --- a/tests/erlang_tests/CMakeLists.txt +++ b/tests/erlang_tests/CMakeLists.txt @@ -388,6 +388,7 @@ compile_erlang(bin2float) compile_erlang(list2float) compile_erlang(test_fp_allocate_heap_zero) +compile_erlang(test_bs_init2_heap_allocation) compile_erlang(improper_concat) compile_erlang(improper_cmp) @@ -828,6 +829,7 @@ add_custom_target(erlang_test_modules DEPENDS list2float.beam test_fp_allocate_heap_zero.beam + test_bs_init2_heap_allocation.beam improper_concat.beam improper_cmp.beam diff --git a/tests/erlang_tests/test_bs_init2_heap_allocation.erl b/tests/erlang_tests/test_bs_init2_heap_allocation.erl new file mode 100644 index 000000000..e1523b243 --- /dev/null +++ b/tests/erlang_tests/test_bs_init2_heap_allocation.erl @@ -0,0 +1,45 @@ +% +% This file is part of AtomVM. +% +% Copyright 2023 Paul Guyot +% +% 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. +% +% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later +% + +-module(test_bs_init2_heap_allocation). + +-export([start/0]). + +start() -> + Display = self(), + disp(Display, 16#00000000), + disp(Display, 16#000000FF), + receive + [{text, 10, 16#000000FF, <<"Test 255">>}] -> ok + end, + 0. + +%% Because of integer_to_binary, the size of the binary is computed +%% with byte_size which is a gc bif. As a consequence, put_tuple2 and put_list +%% are not preceded by a test_heap opcode. Instead, the amount of required +%% memory is encoded in bs_init2. +%% This code crashed when compiled with OTP24 before bs_init2 actually ensured +%% the required amount of words. +disp(Display, Num) -> + Bin = integer_to_binary(Num), + Scene = [ + {text, 10, Num, <<"Test ", Bin/binary>>} + ], + Display ! Scene. diff --git a/tests/test.c b/tests/test.c index fd839e9c5..b9941c668 100644 --- a/tests/test.c +++ b/tests/test.c @@ -414,6 +414,8 @@ struct Test tests[] = { TEST_CASE(test_fp_allocate_heap_zero), + TEST_CASE(test_bs_init2_heap_allocation), + TEST_CASE_EXPECTED(improper_concat, 7), TEST_CASE_EXPECTED(improper_cmp, 3), TEST_CASE_EXPECTED(improper_literal, 3),