From 16d23f7ea9517b8743d754a2f2d4758abf42d60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Fri, 9 Aug 2019 12:59:48 +0200 Subject: erts: Disallow binaries whose size in bits exceeds UWORD_MAX These have never worked in binary matching (including sub-binaries extracted from them) so it's hard to justify their existence. They also make a future migration of binary sizes from bytes to bits problematic, so we may as well change it ahead of time. This is potentially incompatible on 32-bit platforms where a NIF or driver could allocate 512MB+ binaries, but allocations that large should be expected to fail anyway. --- erts/emulator/beam/erl_binary.h | 118 ++++++++++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 42 deletions(-) diff --git a/erts/emulator/beam/erl_binary.h b/erts/emulator/beam/erl_binary.h index c9c047255a..66b59ef1a3 100644 --- a/erts/emulator/beam/erl_binary.h +++ b/erts/emulator/beam/erl_binary.h @@ -351,6 +351,19 @@ erts_free_aligned_binary_bytes(byte* buf) erts_free_aligned_binary_bytes_extra(buf,ERTS_ALC_T_TMP); } +/* A binary's size in bits must fit into a word for matching to work. We used + * to allow creating larger binaries than this, but they acted really strangely + * in Erlang code and were pretty much only usable in drivers and NIFs. + * + * This check also ensures, indirectly, that there won't be an overflow when + * the size is bumped by CHICKEN_PAD and the binary struct itself. */ +#define BINARY_OVERFLOW_CHECK(BYTE_SIZE) \ + do { \ + if (ERTS_UNLIKELY(BYTE_SIZE > ERTS_UWORD_MAX / CHAR_BIT)) { \ + return NULL; \ + } \ + } while(0) + /* Explicit extra bytes allocated to counter buggy drivers. ** These extra bytes where earlier (< R13B04) added by an alignment-bug ** in this code. Do we dare remove this in some major release (R14?) maybe? @@ -364,86 +377,107 @@ erts_free_aligned_binary_bytes(byte* buf) ERTS_GLB_INLINE Binary * erts_bin_drv_alloc_fnf(Uint size) { - Uint bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; Binary *res; + Uint bsize; + + BINARY_OVERFLOW_CHECK(size); + bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; - if (bsize < size) /* overflow */ - return NULL; res = erts_alloc_fnf(ERTS_ALC_T_DRV_BINARY, bsize); ERTS_CHK_BIN_ALIGNMENT(res); + if (res) { - res->orig_size = size; - res->intern.flags = BIN_FLAG_DRV; + res->orig_size = size; + res->intern.flags = BIN_FLAG_DRV; erts_refc_init(&res->intern.refc, 1); } + return res; } ERTS_GLB_INLINE Binary * erts_bin_drv_alloc(Uint size) { - Uint bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; + Binary *res = erts_bin_drv_alloc_fnf(size); + + if (res) { + return res; + } + + erts_alloc_enomem(ERTS_ALC_T_DRV_BINARY, size); +} + +ERTS_GLB_INLINE Binary * +erts_bin_nrml_alloc_fnf(Uint size) +{ Binary *res; + Uint bsize; - if (bsize < size) /* overflow */ - erts_alloc_enomem(ERTS_ALC_T_DRV_BINARY, size); - res = erts_alloc(ERTS_ALC_T_DRV_BINARY, bsize); + BINARY_OVERFLOW_CHECK(size); + bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; + + res = erts_alloc_fnf(ERTS_ALC_T_BINARY, bsize); ERTS_CHK_BIN_ALIGNMENT(res); - res->orig_size = size; - res->intern.flags = BIN_FLAG_DRV; - erts_refc_init(&res->intern.refc, 1); + + if (res) { + res->orig_size = size; + res->intern.flags = 0; + erts_refc_init(&res->intern.refc, 1); + } + return res; } ERTS_GLB_INLINE Binary * erts_bin_nrml_alloc(Uint size) { - Uint bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; - Binary *res; + Binary *res = erts_bin_drv_alloc_fnf(size); - if (bsize < size) /* overflow */ - erts_alloc_enomem(ERTS_ALC_T_BINARY, size); - res = erts_alloc(ERTS_ALC_T_BINARY, bsize); - ERTS_CHK_BIN_ALIGNMENT(res); - res->orig_size = size; - res->intern.flags = 0; - erts_refc_init(&res->intern.refc, 1); - return res; + if (res) { + return res; + } + + erts_alloc_enomem(ERTS_ALC_T_BINARY, size); } ERTS_GLB_INLINE Binary * erts_bin_realloc_fnf(Binary *bp, Uint size) { + ErtsAlcType_t type; Binary *nbp; - Uint bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; - ErtsAlcType_t type = (bp->intern.flags & BIN_FLAG_DRV) ? ERTS_ALC_T_DRV_BINARY - : ERTS_ALC_T_BINARY; + Uint bsize; + + type = (bp->intern.flags & BIN_FLAG_DRV) ? ERTS_ALC_T_DRV_BINARY + : ERTS_ALC_T_BINARY; ASSERT((bp->intern.flags & BIN_FLAG_MAGIC) == 0); - if (bsize < size) /* overflow */ - return NULL; + + BINARY_OVERFLOW_CHECK(size); + bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; + nbp = erts_realloc_fnf(type, (void *) bp, bsize); ERTS_CHK_BIN_ALIGNMENT(nbp); - if (nbp) - nbp->orig_size = size; + + if (nbp) { + nbp->orig_size = size; + } + return nbp; } ERTS_GLB_INLINE Binary * erts_bin_realloc(Binary *bp, Uint size) { - Binary *nbp; - Uint bsize = ERTS_SIZEOF_Binary(size) + CHICKEN_PAD; - ErtsAlcType_t type = (bp->intern.flags & BIN_FLAG_DRV) ? ERTS_ALC_T_DRV_BINARY - : ERTS_ALC_T_BINARY; - ASSERT((bp->intern.flags & BIN_FLAG_MAGIC) == 0); - if (bsize < size) /* overflow */ - erts_realloc_enomem(type, bp, size); - nbp = erts_realloc_fnf(type, (void *) bp, bsize); - if (!nbp) - erts_realloc_enomem(type, bp, bsize); - ERTS_CHK_BIN_ALIGNMENT(nbp); - nbp->orig_size = size; - return nbp; + Binary *nbp = erts_bin_realloc_fnf(bp, size); + + if (nbp) { + return nbp; + } + + if (bp->intern.flags & BIN_FLAG_DRV) { + erts_realloc_enomem(ERTS_ALC_T_DRV_BINARY, bp, size); + } else { + erts_realloc_enomem(ERTS_ALC_T_BINARY, bp, size); + } } ERTS_GLB_INLINE void -- cgit v1.2.3 From 83cd52cf585e37f65f956f18464b8cec98dde469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Fri, 9 Aug 2019 14:24:39 +0200 Subject: erts: Remove size check in bs_start_match The size check is redundant now that all binaries are guaranteed to be small enough that their size in bits fits into a word. --- erts/emulator/beam/bs_instrs.tab | 16 +--------------- erts/emulator/beam/erl_bits.c | 11 +++++------ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/erts/emulator/beam/bs_instrs.tab b/erts/emulator/beam/bs_instrs.tab index bd1ad91e45..4a878ac1db 100644 --- a/erts/emulator/beam/bs_instrs.tab +++ b/erts/emulator/beam/bs_instrs.tab @@ -876,9 +876,7 @@ bs_start_match.execute(Fail, Live, Slots, Dst) { result = erts_bs_start_match_2(c_p, context, slots); HTOP = HEAP_TOP(c_p); HEAP_SPACE_VERIFIED(0); - if (is_non_value(result)) { - $FAIL($Fail); - } + $REFRESH_GEN_DEST(); $Dst = result; } else { @@ -1296,10 +1294,6 @@ i_bs_start_match3_gp.execute(Live, Fail, Dst, Pos) { HTOP = HEAP_TOP(c_p); HEAP_SPACE_VERIFIED(0); - if (ms == NULL) { - $FAIL($Fail); - } - $REFRESH_GEN_DEST(); $Dst = make_matchstate(ms); position = ms->mb.offset; @@ -1348,10 +1342,6 @@ i_bs_start_match3.execute(Live, Fail, Dst) { HTOP = HEAP_TOP(c_p); HEAP_SPACE_VERIFIED(0); - if (ms == NULL) { - $FAIL($Fail); - } - $REFRESH_GEN_DEST(); $Dst = make_matchstate(ms); } else { @@ -1523,10 +1513,6 @@ i_bs_start_match3.execute(Live, Fail, Dst) { HTOP = HEAP_TOP(c_p); HEAP_SPACE_VERIFIED(0); - if (is_non_value(result)) { - $FAIL($Fail); - } - $REFRESH_GEN_DEST(); $Dst = result; } else { diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index f5807d25d7..df4d79651f 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -124,10 +124,10 @@ erts_bs_start_match_2(Process *p, Eterm Binary, Uint Max) ProcBin* pb; ASSERT(is_binary(Binary)); + total_bin_size = binary_size(Binary); - if ((total_bin_size >> (8*sizeof(Uint)-3)) != 0) { - return THE_NON_VALUE; - } + ASSERT(total_bin_size <= ERTS_UWORD_MAX / CHAR_BIT); + NeededSize = ERL_BIN_MATCHSTATE_SIZE(Max); hp = HeapOnlyAlloc(p, NeededSize); ms = (ErlBinMatchState *) hp; @@ -157,10 +157,9 @@ ErlBinMatchState *erts_bs_start_match_3(Process *p, Eterm Binary) ProcBin* pb; ASSERT(is_binary(Binary)); + total_bin_size = binary_size(Binary); - if ((total_bin_size >> (8*sizeof(Uint)-3)) != 0) { - return NULL; - } + ASSERT(total_bin_size <= ERTS_UWORD_MAX / CHAR_BIT); NeededSize = ERL_BIN_MATCHSTATE_SIZE(0); hp = HeapOnlyAlloc(p, NeededSize); -- cgit v1.2.3 From 9f73f98ad6ddf70d30dfc0a1daf7c992dcc5061a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Thu, 8 Aug 2019 10:06:25 +0200 Subject: erts: Create heap binaries in bs_get_binary2 ErlSubBin is a large struct that often dwarfs the region of memory it points at, and it's common for them to refer to a ProcBin which must be kept around as long as the SubBin lives, using up even more heap space and keeping the referenced binary alive regardless of how small the sub-binary is. --- erts/emulator/beam/bs_instrs.tab | 7 ++- erts/emulator/beam/erl_bits.c | 84 ++++++++++++++++++++++----------- erts/emulator/beam/erl_bits.h | 18 ++++++- lib/observer/test/crashdump_helper.erl | 6 +-- lib/stdlib/test/binary_module_SUITE.erl | 32 ++++++------- 5 files changed, 95 insertions(+), 52 deletions(-) diff --git a/erts/emulator/beam/bs_instrs.tab b/erts/emulator/beam/bs_instrs.tab index 4a878ac1db..5f25bc2ad3 100644 --- a/erts/emulator/beam/bs_instrs.tab +++ b/erts/emulator/beam/bs_instrs.tab @@ -149,7 +149,7 @@ i_bs_get_binary_all2.execute(Fail, Live, Unit, Dst) { ErlBinMatchBuffer *_mb; Eterm _result; - $GC_TEST_PRESERVE(ERL_SUB_BIN_SIZE, $Live, context); + $GC_TEST_PRESERVE(EXTRACT_SUB_BIN_HEAP_NEED, $Live, context); _mb = ms_matchbuffer(context); if (((_mb->size - _mb->offset) % $Unit) == 0) { LIGHT_SWAPOUT; @@ -179,7 +179,7 @@ i_bs_get_binary2.execute(Fail, Live, Sz, Flags, Dst) { Eterm _result; Uint _size; $BS_GET_FIELD_SIZE($Sz, (($Flags) >> 3), $FAIL($Fail), _size); - $GC_TEST_PRESERVE(ERL_SUB_BIN_SIZE, $Live, context); + $GC_TEST_PRESERVE(EXTRACT_SUB_BIN_HEAP_NEED, $Live, context); _mb = ms_matchbuffer(context); LIGHT_SWAPOUT; _result = erts_bs_get_binary_2(c_p, _size, $Flags, _mb); @@ -206,8 +206,7 @@ i_bs_get_binary_imm2.fetch(Ctx) { i_bs_get_binary_imm2.execute(Fail, Live, Sz, Flags, Dst) { ErlBinMatchBuffer *_mb; Eterm _result; - $GC_TEST_PRESERVE(heap_bin_size(ERL_ONHEAP_BIN_LIMIT), - $Live, context); + $GC_TEST_PRESERVE(EXTRACT_SUB_BIN_HEAP_NEED, $Live, context); _mb = ms_matchbuffer(context); LIGHT_SWAPOUT; _result = erts_bs_get_binary_2(c_p, $Sz, $Flags, _mb); diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c index df4d79651f..a0edbc81a4 100644 --- a/erts/emulator/beam/erl_bits.c +++ b/erts/emulator/beam/erl_bits.c @@ -458,29 +458,25 @@ erts_bs_get_integer_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuff Eterm erts_bs_get_binary_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer* mb) { - ErlSubBin* sb; + Eterm result; CHECK_MATCH_BUFFER(mb); - if (mb->size - mb->offset < num_bits) { /* Asked for too many bits. */ - return THE_NON_VALUE; + if (mb->size - mb->offset < num_bits) { + /* Asked for too many bits. */ + return THE_NON_VALUE; } /* * From now on, we can't fail. */ - sb = (ErlSubBin *) HeapOnlyAlloc(p, ERL_SUB_BIN_SIZE); - - sb->thing_word = HEADER_SUB_BIN; - sb->orig = mb->orig; - sb->size = BYTE_OFFSET(num_bits); - sb->bitsize = BIT_OFFSET(num_bits); - sb->offs = BYTE_OFFSET(mb->offset); - sb->bitoffs = BIT_OFFSET(mb->offset); - sb->is_writable = 0; + result = erts_extract_sub_binary(&HEAP_TOP(p), + mb->orig, mb->base, + mb->offset, num_bits); + mb->offset += num_bits; - - return make_binary(sb); + + return result; } Eterm @@ -544,21 +540,19 @@ erts_bs_get_float_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer Eterm erts_bs_get_binary_all_2(Process *p, ErlBinMatchBuffer* mb) { - ErlSubBin* sb; - Uint size; + Uint bit_size; + Eterm result; CHECK_MATCH_BUFFER(mb); - size = mb->size-mb->offset; - sb = (ErlSubBin *) HeapOnlyAlloc(p, ERL_SUB_BIN_SIZE); - sb->thing_word = HEADER_SUB_BIN; - sb->size = BYTE_OFFSET(size); - sb->bitsize = BIT_OFFSET(size); - sb->offs = BYTE_OFFSET(mb->offset); - sb->bitoffs = BIT_OFFSET(mb->offset); - sb->is_writable = 0; - sb->orig = mb->orig; - mb->offset = mb->size; - return make_binary(sb); + bit_size = mb->size - mb->offset; + + result = erts_extract_sub_binary(&HEAP_TOP(p), + mb->orig, mb->base, + mb->offset, bit_size); + + mb->offset = mb->size; + + return result; } /**************************************************************** @@ -2096,3 +2090,39 @@ erts_copy_bits(byte* src, /* Base pointer to source. */ } } +Eterm erts_extract_sub_binary(Eterm **hp, Eterm base_bin, byte *base_data, + Uint bit_offset, Uint bit_size) +{ + Uint byte_offset, byte_size; + + ERTS_CT_ASSERT(ERL_SUB_BIN_SIZE <= ERL_ONHEAP_BIN_LIMIT); + + byte_offset = BYTE_OFFSET(bit_offset); + byte_size = BYTE_OFFSET(bit_size); + + if (BIT_OFFSET(bit_size) == 0 && byte_size <= ERL_ONHEAP_BIN_LIMIT) { + ErlHeapBin *hb = (ErlHeapBin*)*hp; + *hp += heap_bin_size(byte_size); + + hb->thing_word = header_heap_bin(byte_size); + hb->size = byte_size; + + copy_binary_to_buffer(hb->data, 0, base_data, bit_offset, bit_size); + + return make_binary(hb); + } else { + ErlSubBin *sb = (ErlSubBin*)*hp; + *hp += ERL_SUB_BIN_SIZE; + + sb->thing_word = HEADER_SUB_BIN; + sb->size = byte_size; + sb->offs = byte_offset; + sb->orig = base_bin; + sb->bitoffs = BIT_OFFSET(bit_offset); + sb->bitsize = BIT_OFFSET(bit_size); + sb->is_writable = 0; + + return make_binary(sb); + } +} + diff --git a/erts/emulator/beam/erl_bits.h b/erts/emulator/beam/erl_bits.h index 50d353e1fa..0b2a6f3760 100644 --- a/erts/emulator/beam/erl_bits.h +++ b/erts/emulator/beam/erl_bits.h @@ -116,7 +116,7 @@ typedef struct erl_bin_match_struct{ do { \ if (BIT_OFFSET(DstBufOffset) == 0 && (SrcBufferOffset == 0) && \ (BIT_OFFSET(NumBits)==0) && (NumBits != 0)) { \ - sys_memcpy(DstBuffer+BYTE_OFFSET(DstBufOffset), \ + sys_memcpy(((byte*)DstBuffer)+BYTE_OFFSET(DstBufOffset), \ SrcBuffer, NBYTES(NumBits)); \ } else { \ erts_copy_bits(SrcBuffer, SrcBufferOffset, 1, \ @@ -150,8 +150,11 @@ void erts_bits_destroy_state(ERL_BITS_PROTO_0); Eterm erts_bs_start_match_2(Process *p, Eterm Bin, Uint Max); ErlBinMatchState *erts_bs_start_match_3(Process *p, Eterm Bin); Eterm erts_bs_get_integer_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer* mb); -Eterm erts_bs_get_binary_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer* mb); Eterm erts_bs_get_float_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer* mb); + +/* These will create heap binaries when appropriate, so they require free space + * up to EXTRACT_SUB_BIN_HEAP_NEED. */ +Eterm erts_bs_get_binary_2(Process *p, Uint num_bits, unsigned flags, ErlBinMatchBuffer* mb); Eterm erts_bs_get_binary_all_2(Process *p, ErlBinMatchBuffer* mb); /* @@ -182,6 +185,17 @@ 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); + +/* 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 */ +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)) + /* * Flags for bs_get_* / bs_put_* / bs_init* instructions. */ diff --git a/lib/observer/test/crashdump_helper.erl b/lib/observer/test/crashdump_helper.erl index 84ed99afa5..10d88c994a 100644 --- a/lib/observer/test/crashdump_helper.erl +++ b/lib/observer/test/crashdump_helper.erl @@ -48,7 +48,7 @@ n1_proc(Creator,_N2,Pid2,Port2,_L) -> Ref = make_ref(), Pid = self(), Bin = list_to_binary(lists:seq(1, 255)), - <<_:2,SubBin:17/binary,_/bits>> = Bin, + <<_:2,SubBin:65/binary,_/bits>> = Bin, register(named_port,Port), @@ -102,7 +102,7 @@ remote_proc(P1,Creator) -> end). create_binaries() -> - Sizes = lists:seq(60, 70) ++ lists:seq(120, 140), + Sizes = lists:seq(100, 120) ++ lists:seq(200, 240), [begin <> = erlang:md5(<>), Data = ((H bsl (8*150)) div (H+7919)), @@ -113,7 +113,7 @@ create_sub_binaries(Bins) -> [create_sub_binary(Bin, Start, LenSub) || Bin <- Bins, Start <- [0,1,2,3,4,5,10,22], - LenSub <- [0,1,2,3,4,6,9]]. + LenSub <- [0,1,2,3,4,6,9,65]]. create_sub_binary(Bin, Start, LenSub) -> Len = byte_size(Bin) - LenSub - Start, diff --git a/lib/stdlib/test/binary_module_SUITE.erl b/lib/stdlib/test/binary_module_SUITE.erl index 9b2033ec4a..be8ab3b98e 100644 --- a/lib/stdlib/test/binary_module_SUITE.erl +++ b/lib/stdlib/test/binary_module_SUITE.erl @@ -716,22 +716,22 @@ referenced(Config) when is_list(Config) -> badarg = ?MASK_ERROR(binary:referenced_byte_size(apa)), badarg = ?MASK_ERROR(binary:referenced_byte_size({})), badarg = ?MASK_ERROR(binary:referenced_byte_size(1)), - A = <<1,2,3>>, - B = binary:copy(A,1000), - 3 = binary:referenced_byte_size(A), - 3000 = binary:referenced_byte_size(B), - <<_:8,C:2/binary>> = A, - 3 = binary:referenced_byte_size(C), - 2 = binary:referenced_byte_size(binary:copy(C)), - <<_:7,D:2/binary,_:1>> = A, - 2 = binary:referenced_byte_size(binary:copy(D)), - 3 = binary:referenced_byte_size(D), - <<_:8,E:2/binary,_/binary>> = B, - 3000 = binary:referenced_byte_size(E), - 2 = binary:referenced_byte_size(binary:copy(E)), - <<_:7,F:2/binary,_:1,_/binary>> = B, - 2 = binary:referenced_byte_size(binary:copy(F)), - 3000 = binary:referenced_byte_size(F), + A = <<0:(1024 * 8)>>, + B = binary:copy(A, 1000), + 1024 = binary:referenced_byte_size(A), + 1024000 = binary:referenced_byte_size(B), + <<_:8,C:1023/binary>> = A, + 1024 = binary:referenced_byte_size(C), + 1023 = binary:referenced_byte_size(binary:copy(C)), + <<_:7,D:1023/binary,_:1>> = A, + 1023 = binary:referenced_byte_size(binary:copy(D)), + 1024 = binary:referenced_byte_size(D), + <<_:8,E:128/binary,_/binary>> = B, + 1024000 = binary:referenced_byte_size(E), + 128 = binary:referenced_byte_size(binary:copy(E)), + <<_:7,F:128/binary,_:1,_/binary>> = B, + 128 = binary:referenced_byte_size(binary:copy(F)), + 1024000 = binary:referenced_byte_size(F), ok. -- cgit v1.2.3 From 1ad01e22988af7c3c81b5c4d41a1e94811c70f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Thu, 8 Aug 2019 13:19:35 +0200 Subject: erts: Create heap binaries in split_binary/2 --- erts/emulator/beam/binary.c | 80 ++++++++++++++++++------------------- erts/emulator/test/binary_SUITE.erl | 17 ++++---- 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/erts/emulator/beam/binary.c b/erts/emulator/beam/binary.c index a18228b84a..4ddf59092a 100644 --- a/erts/emulator/beam/binary.c +++ b/erts/emulator/beam/binary.c @@ -1153,51 +1153,47 @@ BIF_RETTYPE list_to_bitstring_1(BIF_ALIST_1) BIF_RETTYPE split_binary_2(BIF_ALIST_2) { - Uint pos; - ErlSubBin* sb1; - ErlSubBin* sb2; - size_t orig_size; - Eterm orig; - Uint offset; - Uint bit_offset; - Uint bit_size; - Eterm* hp; + size_t orig_size, left_size, right_size; + Uint byte_offset, bit_offset, bit_size; + Uint split_at; + + Eterm *hp, *hp_end; + Eterm left, right; + Eterm real_bin; + Eterm result; + byte *bptr; if (is_not_binary(BIF_ARG_1)) { - goto error; - } - if (!term_to_Uint(BIF_ARG_2, &pos)) { - goto error; - } - if ((orig_size = binary_size(BIF_ARG_1)) < pos) { - goto error; + BIF_ERROR(BIF_P, BADARG); + } else if (!term_to_Uint(BIF_ARG_2, &split_at)) { + BIF_ERROR(BIF_P, BADARG); + } else if ((orig_size = binary_size(BIF_ARG_1)) < split_at) { + BIF_ERROR(BIF_P, BADARG); } - hp = HAlloc(BIF_P, 2*ERL_SUB_BIN_SIZE+3); - ERTS_GET_REAL_BIN(BIF_ARG_1, orig, offset, bit_offset, bit_size); - sb1 = (ErlSubBin *) hp; - sb1->thing_word = HEADER_SUB_BIN; - sb1->size = pos; - sb1->offs = offset; - sb1->orig = orig; - sb1->bitoffs = bit_offset; - sb1->bitsize = 0; - sb1->is_writable = 0; - hp += ERL_SUB_BIN_SIZE; - - sb2 = (ErlSubBin *) hp; - sb2->thing_word = HEADER_SUB_BIN; - sb2->size = orig_size - pos; - sb2->offs = offset + pos; - sb2->orig = orig; - sb2->bitoffs = bit_offset; - sb2->bitsize = bit_size; /* The extra bits go into the second binary. */ - sb2->is_writable = 0; - hp += ERL_SUB_BIN_SIZE; - - return TUPLE2(hp, make_binary(sb1), make_binary(sb2)); - - error: - BIF_ERROR(BIF_P, BADARG); + + left_size = split_at; + right_size = orig_size - split_at; + + ERTS_GET_REAL_BIN(BIF_ARG_1, real_bin, byte_offset, bit_offset, bit_size); + bptr = binary_bytes(real_bin); + + hp = HAlloc(BIF_P, EXTRACT_SUB_BIN_HEAP_NEED * 2 + 3); + hp_end = hp + (EXTRACT_SUB_BIN_HEAP_NEED * 2 + 3); + + left = erts_extract_sub_binary(&hp, real_bin, bptr, + byte_offset * 8 + bit_offset, + left_size * 8); + + right = erts_extract_sub_binary(&hp, real_bin, bptr, + (byte_offset + split_at) * 8 + bit_offset, + right_size * 8 + bit_size); + + result = TUPLE2(hp, left, right); + hp += 3; + + HRelease(BIF_P, hp_end, hp); + + return result; } diff --git a/erts/emulator/test/binary_SUITE.erl b/erts/emulator/test/binary_SUITE.erl index 4fb339926e..fbd1325c3a 100644 --- a/erts/emulator/test/binary_SUITE.erl +++ b/erts/emulator/test/binary_SUITE.erl @@ -1438,7 +1438,8 @@ sleeper() -> gc_test(Config) when is_list(Config) -> %% Note: This test is only relevant for REFC binaries. %% Therefore, we take care that all binaries are REFC binaries. - B = list_to_binary(lists:seq(0, ?heap_binary_size)), + true = 192 > ?heap_binary_size, + B = list_to_binary(lists:seq(1, 192)), Self = self(), F1 = fun() -> gc(), @@ -1447,22 +1448,22 @@ gc_test(Config) when is_list(Config) -> end, F = fun() -> receive go -> ok end, - {binary,[{_,65,1}]} = process_info(self(), binary), + {binary,[{_,192,1}]} = process_info(self(), binary), gc(), - {B1,B2} = my_split_binary(B, 4), + {B1,B2} = my_split_binary(B, 68), gc(), gc(), {binary,L1} = process_info(self(), binary), [Binfo1,Binfo2,Binfo3] = L1, - {_,65,3} = Binfo1 = Binfo2 = Binfo3, - 65 = size(B), - 4 = size(B1), - 61 = size(B2), + {_,192,3} = Binfo1 = Binfo2 = Binfo3, + 192 = size(B), + 68 = size(B1), + 124 = size(B2), F1() end, gc(), gc(), - 65 = size(B), + 192 = size(B), gc_test1(spawn_opt(erlang, apply, [F,[]], [link,{fullsweep_after,0}])). gc_test1(Pid) -> -- cgit v1.2.3 From 9187c2d8b8a3fa4e8b9539d6acdb8966dbbfd04e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Fri, 9 Aug 2019 09:54:40 +0200 Subject: erts: Create heap binaries in binary_part/2-3 --- erts/emulator/beam/erl_bif_binary.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/erts/emulator/beam/erl_bif_binary.c b/erts/emulator/beam/erl_bif_binary.c index 4d6d31cd76..9301b4412b 100644 --- a/erts/emulator/beam/erl_bif_binary.c +++ b/erts/emulator/beam/erl_bif_binary.c @@ -1937,8 +1937,8 @@ BIF_RETTYPE erts_binary_part(Process *p, Eterm binary, Eterm epos, Eterm elen) Uint offset; Uint bit_offset; Uint bit_size; - Eterm* hp; - ErlSubBin* sb; + Eterm *hp, *hp_end; + Eterm result; if (is_not_binary(binary)) { goto badarg; @@ -1970,19 +1970,18 @@ BIF_RETTYPE erts_binary_part(Process *p, Eterm binary, Eterm epos, Eterm elen) goto badarg; } - hp = HeapFragOnlyAlloc(p, ERL_SUB_BIN_SIZE); + hp = HeapFragOnlyAlloc(p, EXTRACT_SUB_BIN_HEAP_NEED); + hp_end = hp + EXTRACT_SUB_BIN_HEAP_NEED; ERTS_GET_REAL_BIN(binary, orig, offset, bit_offset, bit_size); - sb = (ErlSubBin *) hp; - sb->thing_word = HEADER_SUB_BIN; - sb->size = len; - sb->offs = offset + pos; - sb->orig = orig; - sb->bitoffs = bit_offset; - sb->bitsize = 0; - sb->is_writable = 0; - - BIF_RET(make_binary(sb)); + + result = erts_extract_sub_binary(&hp, orig, binary_bytes(orig), + (offset + pos) * 8 + bit_offset, + len * 8); + + HRelease(p, hp_end, hp); + + BIF_RET(result); badarg: BIF_ERROR(p, BADARG); -- cgit v1.2.3 From 9603aed00327d92df7308ff26ba4db79d577c51a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Fri, 9 Aug 2019 09:31:08 +0200 Subject: erts: Create heap binaries in binary:split/2-3 --- erts/emulator/beam/erl_bif_binary.c | 159 ++++++++++++++++++------------------ 1 file changed, 81 insertions(+), 78 deletions(-) diff --git a/erts/emulator/beam/erl_bif_binary.c b/erts/emulator/beam/erl_bif_binary.c index 9301b4412b..b8e56390c1 100644 --- a/erts/emulator/beam/erl_bif_binary.c +++ b/erts/emulator/beam/erl_bif_binary.c @@ -1750,9 +1750,8 @@ static Eterm do_split_single_result(Process *p, Eterm subject, BinaryFindContext Uint offset; Uint bit_offset; Uint bit_size; - ErlSubBin *sb1; - ErlSubBin *sb2; - Eterm *hp; + Uint hp_need; + Eterm *hp, *hp_end; Eterm ret; pos = ff->pos; @@ -1765,57 +1764,58 @@ static Eterm do_split_single_result(Process *p, Eterm subject, BinaryFindContext if (pos == 0) { ret = NIL; } else { - hp = HAlloc(p, (ERL_SUB_BIN_SIZE + 2)); + Eterm extracted; + + hp_need = EXTRACT_SUB_BIN_HEAP_NEED + 2; + + hp = HAlloc(p, hp_need); + hp_end = hp + hp_need; + ERTS_GET_REAL_BIN(subject, orig, offset, bit_offset, bit_size); - sb1 = (ErlSubBin *) hp; - sb1->thing_word = HEADER_SUB_BIN; - sb1->size = pos; - sb1->offs = offset; - sb1->orig = orig; - sb1->bitoffs = bit_offset; - sb1->bitsize = bit_size; - sb1->is_writable = 0; - hp += ERL_SUB_BIN_SIZE; - - ret = CONS(hp, make_binary(sb1), NIL); + extracted = erts_extract_sub_binary(&hp, orig, binary_bytes(orig), + offset * 8 + bit_offset, + pos * 8 + bit_size); + + ret = CONS(hp, extracted, NIL); hp += 2; + + HRelease(p, hp_end, hp); + + return ret; } } else { - if ((ctx->flags & BF_FLAG_SPLIT_TRIM_ALL) && (pos == 0)) { - hp = HAlloc(p, 1 * (ERL_SUB_BIN_SIZE + 2)); - ERTS_GET_REAL_BIN(subject, orig, offset, bit_offset, bit_size); - sb1 = NULL; - } else { - hp = HAlloc(p, 2 * (ERL_SUB_BIN_SIZE + 2)); - ERTS_GET_REAL_BIN(subject, orig, offset, bit_offset, bit_size); - sb1 = (ErlSubBin *) hp; - sb1->thing_word = HEADER_SUB_BIN; - sb1->size = pos; - sb1->offs = offset; - sb1->orig = orig; - sb1->bitoffs = bit_offset; - sb1->bitsize = 0; - sb1->is_writable = 0; - hp += ERL_SUB_BIN_SIZE; - } - - sb2 = (ErlSubBin *) hp; - sb2->thing_word = HEADER_SUB_BIN; - sb2->size = orig_size - pos - len; - sb2->offs = offset + pos + len; - sb2->orig = orig; - sb2->bitoffs = bit_offset; - sb2->bitsize = bit_size; - sb2->is_writable = 0; - hp += ERL_SUB_BIN_SIZE; - - ret = CONS(hp, make_binary(sb2), NIL); - hp += 2; - if (sb1 != NULL) { - ret = CONS(hp, make_binary(sb1), ret); - hp += 2; - } + Eterm first, rest; + + hp_need = (EXTRACT_SUB_BIN_HEAP_NEED + 2) * 2; + + hp = HAlloc(p, hp_need); + hp_end = hp + hp_need; + + ERTS_GET_REAL_BIN(subject, orig, offset, bit_offset, bit_size); + + if ((ctx->flags & BF_FLAG_SPLIT_TRIM_ALL) && (pos == 0)) { + first = NIL; + } else { + first = erts_extract_sub_binary(&hp, orig, binary_bytes(orig), + offset * 8 + bit_offset, + pos * 8); + } + + rest = erts_extract_sub_binary(&hp, orig, binary_bytes(orig), + (offset + pos + len) * 8 + bit_offset, + (orig_size - pos - len) * 8 + bit_size); + + ret = CONS(hp, rest, NIL); + hp += 2; + + if (first != NIL) { + ret = CONS(hp, first, ret); + hp += 2; + } + + HRelease(p, hp_end, hp); } + return ret; } @@ -1829,7 +1829,9 @@ static Eterm do_split_global_result(Process *p, Eterm subject, BinaryFindContext Uint offset; Uint bit_offset; Uint bit_size; - ErlSubBin *sb; + Uint extracted_offset; + Uint extracted_size; + Eterm extracted; Uint do_trim; Sint i; register Uint reds = ctx->reds; @@ -1852,7 +1854,8 @@ static Eterm do_split_global_result(Process *p, Eterm subject, BinaryFindContext *ctxp = ctx; fa = &(ctx->u.fa); } - erts_factory_proc_prealloc_init(&(fa->factory), p, (fa->size + 1) * (ERL_SUB_BIN_SIZE + 2)); + erts_factory_proc_prealloc_init(&(fa->factory), p, (fa->size + 1) * + (EXTRACT_SUB_BIN_HEAP_NEED + 2)); ctx->state = BFResult; } @@ -1871,39 +1874,39 @@ static Eterm do_split_global_result(Process *p, Eterm subject, BinaryFindContext } return THE_NON_VALUE; } - sb = (ErlSubBin *)(fa->factory.hp); - sb->size = fa->end_pos - (fad[i].pos + fad[i].len); - if (!(sb->size == 0 && do_trim)) { - sb->thing_word = HEADER_SUB_BIN; - sb->offs = offset + fad[i].pos + fad[i].len; - sb->orig = orig; - sb->bitoffs = bit_offset; - sb->bitsize = 0; - sb->is_writable = 0; - fa->factory.hp += ERL_SUB_BIN_SIZE; - fa->term = CONS(fa->factory.hp, make_binary(sb), fa->term); - fa->factory.hp += 2; - do_trim &= ~BF_FLAG_SPLIT_TRIM; - } - fa->end_pos = fad[i].pos; + + extracted_offset = (offset + fad[i].pos + fad[i].len) * 8 + bit_offset; + extracted_size = (fa->end_pos - (fad[i].pos + fad[i].len)) * 8; + + if (!(extracted_size == 0 && do_trim)) { + extracted = erts_extract_sub_binary(&fa->factory.hp, orig, + binary_bytes(orig), + extracted_offset, + extracted_size); + fa->term = CONS(fa->factory.hp, extracted, fa->term); + fa->factory.hp += 2; + + do_trim &= ~BF_FLAG_SPLIT_TRIM; + } + + fa->end_pos = fad[i].pos; } fa->head = i; ctx->reds = reds; - sb = (ErlSubBin *)(fa->factory.hp); - sb->size = fad[0].pos; - if (!(sb->size == 0 && do_trim)) { - sb->thing_word = HEADER_SUB_BIN; - sb->offs = offset; - sb->orig = orig; - sb->bitoffs = bit_offset; - sb->bitsize = 0; - sb->is_writable = 0; - fa->factory.hp += ERL_SUB_BIN_SIZE; - fa->term = CONS(fa->factory.hp, make_binary(sb), fa->term); - fa->factory.hp += 2; + extracted_offset = offset * 8 + bit_offset; + extracted_size = fad[0].pos * 8; + + if (!(extracted_size == 0 && do_trim)) { + extracted = erts_extract_sub_binary(&fa->factory.hp, orig, + binary_bytes(orig), + extracted_offset, + extracted_size); + fa->term = CONS(fa->factory.hp, extracted, fa->term); + fa->factory.hp += 2; } + erts_factory_close(&(fa->factory)); return fa->term; -- cgit v1.2.3