Skip to content

Commit

Permalink
Fix allocation with bs_init2 and bs_init_bits (fix atomvm#730)
Browse files Browse the repository at this point in the history 
This is a proper fix of atomvm#730 and replaces atomvm#765

Signed-off-by: Paul Guyot <[email protected]>
  • Loading branch information
@pguyot
pguyot committed Aug 25, 2023
1 parent 9f5ae54 commit 14d589a
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 20 deletions.
50 changes: 30 additions & 20 deletions src/libAtomVM/opcodesswitch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -4528,18 +4529,20 @@ 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) {
TRACE("bs_put_string: Unsupported bit syntax operation. Writing strings must be byte-aligend.\n");
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);
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions tests/erlang_tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions tests/erlang_tests/test_bs_init2_heap_allocation.erl
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
%
% This file is part of AtomVM.
%
% Copyright 2023 Paul Guyot <[email protected]>
%
% 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.
2 changes: 2 additions & 0 deletions tests/test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 14d589a

Please sign in to comment.