From a9ed6ecae18e834ba024d3ee65598812dc68af2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Wed, 23 Aug 2023 04:50:35 +0200 Subject: [PATCH] Fix heap allocation for matching out short bitstrings The runtime system could underestimate the amount of heap space needed for matching out short bitstrings with a size not divisble by 8. That could lead to the runtime system terminating with an "Overrun heap and stack" error. Fixes #7292 --- erts/emulator/beam/emu/generators.tab | 2 +- erts/emulator/beam/erl_bits.c | 16 ++++++++ erts/emulator/beam/erl_bits.h | 17 +++++++-- erts/emulator/beam/jit/arm/instr_bs.cpp | 2 +- erts/emulator/beam/jit/x86/instr_bs.cpp | 2 +- erts/emulator/test/bs_match_bin_SUITE.erl | 46 +++++++++++++++++------ 6 files changed, 68 insertions(+), 17 deletions(-) diff --git a/erts/emulator/beam/emu/generators.tab b/erts/emulator/beam/emu/generators.tab index 1f20a19ecc9e..5e516fc7d8b9 100644 --- a/erts/emulator/beam/emu/generators.tab +++ b/erts/emulator/beam/emu/generators.tab @@ -1100,7 +1100,7 @@ gen.bs_match(Fail, Ctx, N, List) { ASSERT(List[src+3].type == TAG_u); ASSERT(List[src+4].type == TAG_u); size = List[src+3].val * List[src+4].val; - words_needed = heap_bin_size((size + 7) / 8); + words_needed = erts_extracted_binary_size(size); break; case am_integer: ASSERT(List[src+3].type == TAG_u); diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index dee861c5d02b..d993674f7bea 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -2288,6 +2288,22 @@ erts_copy_bits(byte* src, /* Base pointer to source. */ } } +/* + * Calculate sufficient heap space for a binary extracted by + * erts_extract_sub_binary(). + */ +Uint erts_extracted_binary_size(Uint bit_size) +{ + Uint byte_size = BYTE_OFFSET(bit_size); + ERTS_CT_ASSERT(ERL_SUB_BIN_SIZE <= ERL_ONHEAP_BIN_LIMIT); + + if (BIT_OFFSET(bit_size) == 0 && byte_size <= ERL_ONHEAP_BIN_LIMIT) { + return heap_bin_size(byte_size); + } else { + return ERL_SUB_BIN_SIZE; + } +} + Eterm erts_extract_sub_binary(Eterm **hp, Eterm base_bin, byte *base_data, Uint bit_offset, Uint bit_size) { diff --git a/erts/emulator/beam/erl_bits.h b/erts/emulator/beam/erl_bits.h index 482d99497f8c..2e83b462de68 100644 --- a/erts/emulator/beam/erl_bits.h +++ b/erts/emulator/beam/erl_bits.h @@ -196,16 +196,27 @@ void erts_copy_bits(byte* src, size_t soffs, int sdir, byte* dst, size_t doffs,int ddir, size_t n); int erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t size); +/* + * Calculate the heap space for a binary extracted by + * erts_extract_sub_binary(). + */ +Uint erts_extracted_binary_size(Uint bit_size); /* Extracts a region from base_bin as a sub-binary or heap binary, whichever * is the most appropriate. * - * The caller must ensure that there's enough free space at *hp */ + * The caller must ensure that there's enough free space at *hp by using + * erts_extracted_binary_size(). + * */ Eterm erts_extract_sub_binary(Eterm **hp, Eterm base_bin, byte *base_data, Uint bit_offset, Uint num_bits); -/* Pessimistic estimate of the words required for erts_extract_sub_binary */ -#define EXTRACT_SUB_BIN_HEAP_NEED (heap_bin_size(ERL_ONHEAP_BIN_LIMIT)) +/* + * Conservative estimate of the number of words required for + * erts_extract_sub_binary() when the number of bits is unknown. + */ +#define EXTRACT_SUB_BIN_HEAP_NEED \ + (MAX(ERL_SUB_BIN_SIZE, heap_bin_size(ERL_ONHEAP_BIN_LIMIT))) /* * Flags for bs_create_bin / bs_get_* / bs_put_* / bs_init* instructions. diff --git a/erts/emulator/beam/jit/arm/instr_bs.cpp b/erts/emulator/beam/jit/arm/instr_bs.cpp index d7e8f70d8378..e9d331a37b0a 100644 --- a/erts/emulator/beam/jit/arm/instr_bs.cpp +++ b/erts/emulator/beam/jit/arm/instr_bs.cpp @@ -3700,7 +3700,7 @@ static std::vector opt_bsm_segments( } break; case BsmSegment::action::GET_BINARY: - heap_need += heap_bin_size((seg.size + 7) / 8); + heap_need += erts_extracted_binary_size(seg.size); break; case BsmSegment::action::GET_TAIL: heap_need += EXTRACT_SUB_BIN_HEAP_NEED; diff --git a/erts/emulator/beam/jit/x86/instr_bs.cpp b/erts/emulator/beam/jit/x86/instr_bs.cpp index 39dfb64f8f7c..a4c801d43d7b 100644 --- a/erts/emulator/beam/jit/x86/instr_bs.cpp +++ b/erts/emulator/beam/jit/x86/instr_bs.cpp @@ -3829,7 +3829,7 @@ static std::vector opt_bsm_segments( } break; case BsmSegment::action::GET_BINARY: - heap_need += heap_bin_size((seg.size + 7) / 8); + heap_need += erts_extracted_binary_size(seg.size); break; case BsmSegment::action::GET_TAIL: heap_need += EXTRACT_SUB_BIN_HEAP_NEED; diff --git a/erts/emulator/test/bs_match_bin_SUITE.erl b/erts/emulator/test/bs_match_bin_SUITE.erl index c29fbd6be764..0ea354b01ee1 100644 --- a/erts/emulator/test/bs_match_bin_SUITE.erl +++ b/erts/emulator/test/bs_match_bin_SUITE.erl @@ -24,7 +24,7 @@ init_per_group/2,end_per_group/2, byte_split_binary/1,bit_split_binary/1,match_huge_bin/1, bs_match_string_edge_case/1,contexts/1, - empty_binary/1]). + empty_binary/1,small_bitstring/1]). -include_lib("common_test/include/ct.hrl"). @@ -32,7 +32,8 @@ suite() -> [{ct_hooks,[ts_install_cth]}]. all() -> [byte_split_binary, bit_split_binary, match_huge_bin, - bs_match_string_edge_case, contexts, empty_binary]. + bs_match_string_edge_case, contexts, empty_binary, + small_bitstring]. groups() -> []. @@ -113,14 +114,6 @@ bits_to_list([], _) -> []. mkbin(L) when is_list(L) -> list_to_binary(L). -make_unaligned_sub_binary(Bin0) -> - Bin1 = <<0:3,Bin0/binary,31:5>>, - Sz = size(Bin0), - <<0:3,Bin:Sz/binary,31:5>> = id(Bin1), - Bin. - -id(I) -> I. - match_huge_bin(Config) when is_list(Config) -> Bin = <<0:(1 bsl 27),13:8>>, skip_huge_bin_1(1 bsl 27, Bin), @@ -280,6 +273,37 @@ do_empty_binary(0) -> do_empty_binary(N) -> %% The new bs_match instruction would use more heap space %% than reserved when matching out an empty binary. - <> = <<>>, + <> = id(<<>>), [0|do_empty_binary(N-1)]. +small_bitstring(_Config) -> + %% GH-7292: The new bs_match instruction would reserve insufficient + %% heap space for small bitstrings. + rand_seed(), + Bin = rand:bytes(10_000), + ok = small_bitstring_1(id(Bin), id(Bin)). + +small_bitstring_1(<>, + <>) -> + small_bitstring_1(As0, As1); +small_bitstring_1(<<>>, <<>>) -> + ok. + +%%% +%%% Common utilities. +%%% + +rand_seed() -> + rand:seed(default), + io:format("\n*** rand:export_seed() = ~w\n\n", [rand:export_seed()]), + ok. + +make_unaligned_sub_binary(Bin0) -> + Bin1 = <<0:3,Bin0/binary,31:5>>, + Sz = size(Bin0), + <<0:3,Bin:Sz/binary,31:5>> = id(Bin1), + Bin. + +id(I) -> I.