Skip to content

Commit

Permalink
Merge pull request #1220 from pguyot/w26/fix-memory-estimate-fun
Browse files Browse the repository at this point in the history
Fix a crash caused by `memory_estimate_usage` failing with some funcs

Functions created with make_fun* opcodes are boxed terms containing terms that
are copied. However, `memory_estimate_usage` did not take them into account.
As a consequence, a crash could occur when such functions were sent in messages
as the copy would overflow a heap allocated with the estimate.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
  • Loading branch information
bettio committed Jun 28, 2024
2 parents 699d2c9 + 10d0bba commit 882bf65
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ in ESP-IDF Kconfig options.
- Fix creation of multiple links for the same process and not removing link at trapped exits.
See issue [#1193](https://github.com/atomvm/AtomVM/issues/1193).
- Fix error that is raised when a function is undefined
- Fix a bug that could yield crashes when functions are sent in messages

## [0.6.2] - 25-05-2024

Expand Down
35 changes: 25 additions & 10 deletions src/libAtomVM/memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,21 +527,36 @@ unsigned long memory_estimate_usage(term t)
t = term_nil();
}

} else if (term_is_function(t)) {
int boxed_size = term_boxed_size(t);
acc += boxed_size + 1;
const term *boxed_value = term_to_const_term_ptr(t);

// We skip the first two elements:
// First is either a module atom or a pointer to a Module
// Second is either a function atom or a function index
// Third would be arity as a term int (external function) or
// the first argument (if built with make_fun3) which we should
// estimate.
for (int i = 2; i < boxed_size; i++) {
if (UNLIKELY(temp_stack_push(&temp_stack, boxed_value[i + 1]) != TempStackOk)) {
// TODO: handle failed malloc
AVM_ABORT();
}
}
t = boxed_value[2];

} else if (term_is_sub_binary(t)) {
acc += term_boxed_size(t) + 1;
t = term_get_sub_binary_ref(t);

} else if (term_is_boxed(t)) {
// Default type of boxed terms
acc += term_boxed_size(t) + 1;
if (term_is_sub_binary(t)) {
t = term_get_sub_binary_ref(t);
} else {
t = temp_stack_pop(&temp_stack);
}
t = temp_stack_pop(&temp_stack);

} else {
fprintf(stderr, "bug: found unknown term type: 0x%" TERM_X_FMT "\n", t);
if (term_is_boxed(t)) {
const term *boxed_value = term_to_const_term_ptr(t);
int boxed_size = term_boxed_size(t) + 1;
fprintf(stderr, "boxed header: 0x%" TERM_X_FMT ", size: %i\n", boxed_value[0], boxed_size);
}
AVM_ABORT();
}
}
Expand Down
24 changes: 22 additions & 2 deletions tests/erlang_tests/test_make_fun3.erl
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,38 @@ start() ->
make_config(A, B, C) ->
erlang:garbage_collect(self()),

Z1 = maketuple(1, 2, 3),
Z2 = maketuple(Z1, 1, 2),
Z3 = maketuple(Z1, Z2, 1),
% With OTP24+, this is implemented with make_fun3 which optimizes allocation.
X = [
{fun() -> 0 end, 1, 2, 3, 4},
% env = 1
{fun() -> A end, 1, 2, 3, 4},
% env = 2
{fun() -> A + B end, 1, 2, 3, 4, 5}
{fun() -> A + B end, 1, 2, 3, 4, 5},
% env = 3, with a term that takes some space
fun() -> sumtuple(Z1) + sumtuple(Z2) + sumtuple(Z3) end
],
maketuple(A, B, C, X).

% Make a copy sending this as a message, to ensure we know how to copy
% terms created by make_fun3
Y = send_self(X),
maketuple(A, B, C, Y).

maketuple(A, B, C) -> {A, B, C}.

maketuple(A, B, C, _D) -> {A, B, C}.

sumtuple({A, B, C}) -> A + B + C.

send_self(X) ->
Ref = make_ref(),
self() ! {Ref, X},
receive_self(Ref).

receive_self(Ref) ->
erlang:garbage_collect(self()),
receive
{Ref, Y} -> Y
end.

0 comments on commit 882bf65

Please sign in to comment.