Skip to content

Commit 0b4fd84

Browse files
committed
Fix crash related to make_fun3 OTP24 opcode
As reported by @muromec and with provided test case, fix a bug where a heap overflow would occur because allocation lists were poorly decoded. Also remove unnecessary calls to `memory_ensure_free` for make_fun3 and update_record opcodes that are marked as generating allocations in beam_ssa_codegen. Signed-off-by: Paul Guyot <[email protected]>
1 parent 42de0ad commit 0b4fd84

File tree

5 files changed

+68
-16
lines changed

5 files changed

+68
-16
lines changed

src/libAtomVM/opcodesswitch.h

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,8 @@ typedef union
391391
DECODE_LITERAL(allocator_size, code_chunk, base_index, off); \
392392
if (allocator_tag == COMPACT_EXTENDED_ALLOCATOR_LIST_TAG_FLOATS) { \
393393
allocator_size *= FLOAT_SIZE; \
394+
} else if (allocator_tag == COMPACT_EXTENDED_ALLOCATOR_LIST_TAG_FUNS) { \
395+
allocator_size *= BOXED_FUN_SIZE; \
394396
} \
395397
need += allocator_size; \
396398
} \
@@ -654,6 +656,8 @@ typedef union
654656
DECODE_LITERAL(allocator_size, code_chunk, base_index, off); \
655657
if (allocator_tag == COMPACT_EXTENDED_ALLOCATOR_LIST_TAG_FLOATS) { \
656658
allocator_size *= FLOAT_SIZE; \
659+
} else if (allocator_tag == COMPACT_EXTENDED_ALLOCATOR_LIST_TAG_FUNS) { \
660+
allocator_size *= BOXED_FUN_SIZE; \
657661
} \
658662
need += allocator_size; \
659663
} \
@@ -1132,13 +1136,13 @@ term make_fun(Context *ctx, const Module *mod, int fun_index)
11321136
{
11331137
uint32_t n_freeze = module_get_fun_freeze(mod, fun_index);
11341138

1135-
int size = 2 + n_freeze;
1136-
if (memory_ensure_free(ctx, size + 1) != MEMORY_GC_OK) {
1139+
int size = BOXED_FUN_SIZE + n_freeze;
1140+
if (memory_ensure_free(ctx, size) != MEMORY_GC_OK) {
11371141
return term_invalid_term();
11381142
}
1139-
term *boxed_func = memory_heap_alloc(ctx, size + 1);
1143+
term *boxed_func = memory_heap_alloc(ctx, size);
11401144

1141-
boxed_func[0] = (size << 6) | TERM_BOXED_FUN;
1145+
boxed_func[0] = ((size - 1) << 6) | TERM_BOXED_FUN;
11421146
boxed_func[1] = (term) mod;
11431147
boxed_func[2] = term_from_int(fun_index);
11441148

@@ -6117,22 +6121,20 @@ static bool maybe_call_native(Context *ctx, AtomString module_name, AtomString f
61176121
dreg_type_t dreg_type;
61186122
DECODE_DEST_REGISTER(dreg, dreg_type, code, i, next_off);
61196123
DECODE_EXTENDED_LIST_TAG(code, i, next_off);
6120-
uint32_t size;
6121-
DECODE_LITERAL(size, code, i, next_off)
6122-
TRACE("make_fun3/3, fun_index=%i dreg=%c%i arity=%i\n", fun_index, T_DEST_REG(dreg_type, dreg), size);
6124+
uint32_t numfree;
6125+
DECODE_LITERAL(numfree, code, i, next_off)
6126+
TRACE("make_fun3/3, fun_index=%i dreg=%c%i numfree=%i\n", fun_index, T_DEST_REG(dreg_type, dreg), numfree);
61236127

61246128
#ifdef IMPL_EXECUTE_LOOP
6125-
if (memory_ensure_free(ctx, size + 3) != MEMORY_GC_OK) {
6126-
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
6127-
}
6128-
term *boxed_func = memory_heap_alloc(ctx, size + 3);
6129+
size_t size = numfree + BOXED_FUN_SIZE;
6130+
term *boxed_func = memory_heap_alloc(ctx, size);
61296131

6130-
boxed_func[0] = ((size + 2) << 6) | TERM_BOXED_FUN;
6132+
boxed_func[0] = ((size - 1) << 6) | TERM_BOXED_FUN;
61316133
boxed_func[1] = (term) mod;
61326134
boxed_func[2] = term_from_int(fun_index);
61336135
#endif
61346136

6135-
for (uint32_t j = 0; j < size; j++) {
6137+
for (uint32_t j = 0; j < numfree; j++) {
61366138
term arg;
61376139
DECODE_COMPACT_TERM(arg, code, i, next_off);
61386140
#ifdef IMPL_EXECUTE_LOOP
@@ -6543,9 +6545,6 @@ static bool maybe_call_native(Context *ctx, AtomString module_name, AtomString f
65436545
DECODE_LITERAL(size, code, i, next_off);
65446546
#ifdef IMPL_EXECUTE_LOOP
65456547
term dst;
6546-
if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(size)) != MEMORY_GC_OK)) {
6547-
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
6548-
}
65496548
dst = term_alloc_tuple(size, ctx);
65506549
#endif
65516550
term src;

src/libAtomVM/term.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ extern "C" {
7777
#define FUNCTION_REFERENCE_SIZE 4
7878
#define BOXED_INT_SIZE (BOXED_TERMS_REQUIRED_FOR_INT + 1)
7979
#define BOXED_INT64_SIZE (BOXED_TERMS_REQUIRED_FOR_INT64 + 1)
80+
#define BOXED_FUN_SIZE 3
8081
#define FLOAT_SIZE (sizeof(float_term_t) / sizeof(term) + 1)
8182
#define REF_SIZE ((int) ((sizeof(uint64_t) / sizeof(term)) + 1))
8283
#define TUPLE_SIZE(elems) ((int) (elems + 1))

tests/erlang_tests/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ compile_erlang(test_funs9)
169169
compile_erlang(test_funs10)
170170
compile_erlang(test_funs11)
171171

172+
compile_erlang(test_make_fun3)
173+
172174
compile_erlang(complex_struct_size0)
173175
compile_erlang(complex_struct_size1)
174176
compile_erlang(complex_struct_size2)
@@ -583,6 +585,8 @@ add_custom_target(erlang_test_modules DEPENDS
583585
test_funs10.beam
584586
test_funs11.beam
585587

588+
test_make_fun3.beam
589+
586590
complex_struct_size0.beam
587591
complex_struct_size1.beam
588592
complex_struct_size2.beam
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
%
2+
% This file is part of AtomVM.
3+
%
4+
% Copyright 2023 Illya Petrov <[email protected]>
5+
%
6+
% Licensed under the Apache License, Version 2.0 (the "License");
7+
% you may not use this file except in compliance with the License.
8+
% You may obtain a copy of the License at
9+
%
10+
% http://www.apache.org/licenses/LICENSE-2.0
11+
%
12+
% Unless required by applicable law or agreed to in writing, software
13+
% distributed under the License is distributed on an "AS IS" BASIS,
14+
% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
% See the License for the specific language governing permissions and
16+
% limitations under the License.
17+
%
18+
% SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
19+
%
20+
-module(test_make_fun3).
21+
22+
-export([start/0, maketuple/3, sumtuple/1]).
23+
24+
start() ->
25+
erlang:garbage_collect(self()),
26+
X = make_config(1, 2, 3),
27+
6 = sumtuple(X),
28+
0.
29+
30+
make_config(A, B, C) ->
31+
erlang:garbage_collect(self()),
32+
33+
% With OTP24+, this is implemented with make_fun3 which optimizes allocation.
34+
X = [
35+
{fun() -> 0 end, 1, 2, 3, 4},
36+
% env = 1
37+
{fun() -> A end, 1, 2, 3, 4},
38+
% env = 2
39+
{fun() -> A + B end, 1, 2, 3, 4, 5}
40+
],
41+
maketuple(A, B, C, X).
42+
43+
maketuple(A, B, C) -> {A, B, C}.
44+
45+
maketuple(A, B, C, _D) -> {A, B, C}.
46+
47+
sumtuple({A, B, C}) -> A + B + C.

tests/test.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ struct Test tests[] = {
189189
TEST_CASE_EXPECTED(test_funs9, 3555),
190190
TEST_CASE_EXPECTED(test_funs10, 6817),
191191
TEST_CASE_EXPECTED(test_funs11, 817),
192+
TEST_CASE(test_make_fun3),
192193

193194
TEST_CASE(nested_list_size0),
194195
TEST_CASE_EXPECTED(nested_list_size1, 2),

0 commit comments

Comments
 (0)