Skip to content

Commit

Permalink
Fix heap allocation for matching out short bitstrings
Browse files Browse the repository at this point in the history
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 erlang#7292
  • Loading branch information
bjorng committed Aug 23, 2023
1 parent d051172 commit c2f575e
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 17 deletions.
2 changes: 1 addition & 1 deletion erts/emulator/beam/emu/generators.tab
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 16 additions & 0 deletions erts/emulator/beam/erl_bits.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
17 changes: 14 additions & 3 deletions erts/emulator/beam/erl_bits.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/beam/jit/arm/instr_bs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3700,7 +3700,7 @@ static std::vector<BsmSegment> 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;
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/beam/jit/x86/instr_bs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3829,7 +3829,7 @@ static std::vector<BsmSegment> 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;
Expand Down
47 changes: 36 additions & 11 deletions erts/emulator/test/bs_match_bin_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@
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").

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() ->
[].
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -280,6 +273,38 @@ 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.
<<V1:0/bits, V1:0/bitstring, V2:0/bytes, V2:0/bits>> = <<>>,
<<V1:0/bits, V1:0/bitstring, V2:0/bytes, V2:0/bits>> = 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(default),
io:format("\n*** rand:export_seed() = ~w\n\n", [rand:export_seed()]),
Bin = rand:bytes(10_000),
ok = small_bitstring_1(id(Bin), id(Bin)).

small_bitstring_1(<<A1:1/bits,A2:1/bits,A3:2/bits,
A4:3/bits,A5:1/bits,As0/binary>>,
<<A1:1/bits,A2:1/bits,A3:2/bits,
A4:3/bits,A5:1/bits,As1/binary>>) ->
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.

0 comments on commit c2f575e

Please sign in to comment.