aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Högberg <[email protected]>2018-02-19 15:35:18 +0100
committerJohn Högberg <[email protected]>2018-02-19 15:35:18 +0100
commit474b2ef465cddf41d311464c48bf99dd007f270f (patch)
treef5fe16b323684a9526fb2b70f380674ae98a82d6
parentb64db2aa782e17982503365ae5cdfbf4edcd633a (diff)
parentc47c8ed7da8c222c5d3b99d0f109fe126a86c632 (diff)
downloadotp-474b2ef465cddf41d311464c48bf99dd007f270f.tar.gz
otp-474b2ef465cddf41d311464c48bf99dd007f270f.tar.bz2
otp-474b2ef465cddf41d311464c48bf99dd007f270f.zip
Merge branch 'john/erts/fix-iovec-unaligned-binaries/OTP-14921' into maint
-rw-r--r--erts/emulator/beam/erl_io_queue.c31
-rw-r--r--erts/emulator/beam/erl_nif.c124
-rw-r--r--erts/emulator/test/iovec_SUITE.erl39
-rw-r--r--erts/emulator/test/nif_SUITE.erl30
4 files changed, 144 insertions, 80 deletions
diff --git a/erts/emulator/beam/erl_io_queue.c b/erts/emulator/beam/erl_io_queue.c
index 190ba6bbb9..990c0c1cac 100644
--- a/erts/emulator/beam/erl_io_queue.c
+++ b/erts/emulator/beam/erl_io_queue.c
@@ -801,12 +801,11 @@ static Eterm iol2v_make_sub_bin(iol2v_state_t *state, Eterm bin_term,
ERTS_GET_REAL_BIN(bin_term, orig_pb_term,
byte_offset, bit_offset, bit_size);
- (void)bit_offset;
- (void)bit_size;
+ ASSERT(bit_size == 0);
sb->thing_word = HEADER_SUB_BIN;
+ sb->bitoffs = bit_offset;
sb->bitsize = 0;
- sb->bitoffs = 0;
sb->orig = orig_pb_term;
sb->is_writable = 0;
@@ -984,7 +983,7 @@ static int iol2v_append_binary(iol2v_state_t *state, Eterm bin_term) {
parent_header = binary_val(parent_binary);
binary_size = binary_size(bin_term);
- if (bit_offset != 0 || bit_size != 0) {
+ if (bit_size != 0) {
return 0;
} else if (binary_size == 0) {
state->bytereds_spent += 1;
@@ -1026,8 +1025,16 @@ static int iol2v_append_binary(iol2v_state_t *state, Eterm bin_term) {
* then just copy it into the accumulator. */
iol2v_expand_acc(state, binary_size);
- sys_memcpy(&(state->acc)->orig_bytes[state->acc_size],
- binary_data, binary_size);
+ if (ERTS_LIKELY(bit_offset == 0)) {
+ sys_memcpy(&(state->acc)->orig_bytes[state->acc_size],
+ binary_data, binary_size);
+ } else {
+ ASSERT(binary_size <= ERTS_UWORD_MAX / 8);
+
+ erts_copy_bits(binary_data, bit_offset, 1,
+ (byte*)&(state->acc)->orig_bytes[state->acc_size], 0, 1,
+ binary_size * 8);
+ }
state->acc_size += binary_size;
} else {
@@ -1038,8 +1045,16 @@ static int iol2v_append_binary(iol2v_state_t *state, Eterm bin_term) {
iol2v_expand_acc(state, spill);
- sys_memcpy(&(state->acc)->orig_bytes[state->acc_size],
- binary_data, spill);
+ if (ERTS_LIKELY(bit_offset == 0)) {
+ sys_memcpy(&(state->acc)->orig_bytes[state->acc_size],
+ binary_data, spill);
+ } else {
+ ASSERT(binary_size <= ERTS_UWORD_MAX / 8);
+
+ erts_copy_bits(binary_data, bit_offset, 1,
+ (byte*)&(state->acc)->orig_bytes[state->acc_size], 0, 1,
+ spill * 8);
+ }
state->acc_size += spill;
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index f67b67325d..b96db5d054 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -3396,8 +3396,8 @@ typedef struct {
Eterm sublist_start;
Eterm sublist_end;
- UWord offheap_size;
- UWord onheap_size;
+ UWord referenced_size;
+ UWord copied_size;
UWord iovec_len;
} iovec_slice_t;
@@ -3407,16 +3407,16 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
result->sublist_start = list;
result->sublist_length = 0;
- result->offheap_size = 0;
- result->onheap_size = 0;
+ result->referenced_size = 0;
+ result->copied_size = 0;
result->iovec_len = 0;
lookahead = result->sublist_start;
while (is_list(lookahead)) {
- Eterm *binary_header, binary;
+ UWord byte_size;
+ Eterm binary;
Eterm *cell;
- UWord size;
cell = list_val(lookahead);
binary = CAR(cell);
@@ -3425,35 +3425,36 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
return 0;
}
- size = binary_size(binary);
- binary_header = binary_val(binary);
+ byte_size = binary_size(binary);
- if (size > 0) {
- /* If we're a sub-binary we'll need to check our underlying binary
- * to determine whether we're on-heap or not. */
- if (thing_subtag(*binary_header) == SUB_BINARY_SUBTAG) {
- ErlSubBin *sb = (ErlSubBin*)binary_header;
+ if (byte_size > 0) {
+ int bit_offset, bit_size;
+ Eterm parent_binary;
+ UWord byte_offset;
- /* Reject bitstrings */
- if((sb->bitoffs + sb->bitsize) > 0) {
- return 0;
- }
+ int requires_copying;
- ASSERT(size <= binary_size(sb->orig));
- binary_header = binary_val(sb->orig);
+ ERTS_GET_REAL_BIN(binary, parent_binary, byte_offset,
+ bit_offset, bit_size);
+
+ (void)byte_offset;
+
+ if (bit_size != 0) {
+ return 0;
}
- if (thing_subtag(*binary_header) == HEAP_BINARY_SUBTAG) {
- ASSERT(size <= ERL_ONHEAP_BIN_LIMIT);
+ /* If we're unaligned or an on-heap binary we'll need to copy
+ * ourselves over to a temporary buffer. */
+ requires_copying = (bit_offset != 0) ||
+ thing_subtag(*binary_val(parent_binary)) == HEAP_BINARY_SUBTAG;
- result->iovec_len += 1;
- result->onheap_size += size;
+ if (requires_copying) {
+ result->copied_size += byte_size;
} else {
- ASSERT(thing_subtag(*binary_header) == REFC_BINARY_SUBTAG);
-
- result->iovec_len += 1 + size / MAX_SYSIOVEC_IOVLEN;
- result->offheap_size += size;
+ result->referenced_size += byte_size;
}
+
+ result->iovec_len += 1 + byte_size / MAX_SYSIOVEC_IOVLEN;
}
result->sublist_length += 1;
@@ -3473,7 +3474,9 @@ static int examine_iovec_term(Eterm list, UWord max_length, iovec_slice_t *resul
return 1;
}
-static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
+static void marshal_iovec_binary(Eterm binary, ErlNifBinary *copy_buffer,
+ UWord *copy_offset, ErlNifBinary *result) {
+
Eterm *parent_header;
Eterm parent_binary;
@@ -3484,6 +3487,8 @@ static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
ERTS_GET_REAL_BIN(binary, parent_binary, byte_offset, bit_offset, bit_size);
+ ASSERT(bit_size == 0);
+
parent_header = binary_val(parent_binary);
result->size = binary_size(binary);
@@ -3510,24 +3515,50 @@ static void inspect_raw_binary_data(Eterm binary, ErlNifBinary *result) {
result->data = &((unsigned char*)&hb->data)[byte_offset];
result->ref_bin = NULL;
}
+
+ /* If this isn't an *aligned* refc binary, copy its contents to the buffer
+ * and reference that instead. */
+
+ if (result->ref_bin == NULL || bit_offset != 0) {
+ ASSERT(result->size <= (copy_buffer->size - *copy_offset));
+
+ if (bit_offset == 0) {
+ sys_memcpy(&copy_buffer->data[*copy_offset],
+ result->data, result->size);
+ } else {
+ erts_copy_bits(result->data, bit_offset, 1,
+ (byte*)&copy_buffer->data[*copy_offset], 0, 1,
+ result->size * 8);
+ }
+
+ result->data = &copy_buffer->data[*copy_offset];
+ result->ref_bin = copy_buffer->ref_bin;
+
+ *copy_offset += result->size;
+ }
}
static int fill_iovec_with_slice(ErlNifEnv *env,
iovec_slice_t *slice,
ErlNifIOVec *iovec) {
- UWord onheap_offset, iovec_idx;
- ErlNifBinary onheap_data;
+ UWord copy_offset, iovec_idx;
+ ErlNifBinary copy_buffer;
Eterm sublist_iterator;
- /* Set up a common refc binary for all on-heap binaries. */
- if (slice->onheap_size > 0) {
- if (!enif_alloc_binary(slice->onheap_size, &onheap_data)) {
+ /* Set up a common refc binary for all on-heap and unaligned binaries. */
+ if (slice->copied_size > 0) {
+ if (!enif_alloc_binary(slice->copied_size, &copy_buffer)) {
return 0;
}
+ } else {
+#ifdef DEBUG
+ copy_buffer.data = NULL;
+ copy_buffer.size = 0;
+#endif
}
sublist_iterator = slice->sublist_start;
- onheap_offset = 0;
+ copy_offset = 0;
iovec_idx = 0;
while (sublist_iterator != slice->sublist_end) {
@@ -3535,20 +3566,7 @@ static int fill_iovec_with_slice(ErlNifEnv *env,
Eterm *cell;
cell = list_val(sublist_iterator);
- inspect_raw_binary_data(CAR(cell), &raw_data);
-
- /* If this isn't a refc binary, copy its contents to the onheap buffer
- * and reference that instead. */
- if (raw_data.size > 0 && raw_data.ref_bin == NULL) {
- ASSERT(onheap_offset < onheap_data.size);
- ASSERT(slice->onheap_size > 0);
-
- sys_memcpy(&onheap_data.data[onheap_offset],
- raw_data.data, raw_data.size);
-
- raw_data.data = &onheap_data.data[onheap_offset];
- raw_data.ref_bin = onheap_data.ref_bin;
- }
+ marshal_iovec_binary(CAR(cell), &copy_buffer, &copy_offset, &raw_data);
while (raw_data.size > 0) {
UWord chunk_len = MIN(raw_data.size, MAX_SYSIOVEC_IOVLEN);
@@ -3579,16 +3597,16 @@ static int fill_iovec_with_slice(ErlNifEnv *env,
erts_refc_inc(&refc_binary->intern.refc, 1);
}
- if (slice->onheap_size > 0) {
+ if (slice->copied_size > 0) {
/* Transfer ownership to the iovec; we've taken references to it in
* the above loop. */
- enif_release_binary(&onheap_data);
+ enif_release_binary(&copy_buffer);
}
} else {
- if (slice->onheap_size > 0) {
+ if (slice->copied_size > 0) {
/* Attach the binary to our environment and let the GC take care of
* it after returning. */
- enif_make_binary(env, &onheap_data);
+ enif_make_binary(env, &copy_buffer);
}
}
@@ -3635,7 +3653,7 @@ static int create_iovec_from_slice(ErlNifEnv *env,
iovec->flags = 0;
}
- iovec->size = slice->offheap_size + slice->onheap_size;
+ iovec->size = slice->referenced_size + slice->copied_size;
iovec->iovcnt = slice->iovec_len;
if(!fill_iovec_with_slice(env, slice, iovec)) {
diff --git a/erts/emulator/test/iovec_SUITE.erl b/erts/emulator/test/iovec_SUITE.erl
index 49dc64b0d2..76adbe4b5e 100644
--- a/erts/emulator/test/iovec_SUITE.erl
+++ b/erts/emulator/test/iovec_SUITE.erl
@@ -25,7 +25,7 @@
-export([integer_lists/1, binary_lists/1, empty_lists/1, empty_binary_lists/1,
mixed_lists/1, improper_lists/1, illegal_lists/1, cons_bomb/1,
sub_binary_lists/1, iolist_to_iovec_idempotence/1,
- iolist_to_iovec_correctness/1]).
+ iolist_to_iovec_correctness/1, unaligned_sub_binaries/1]).
-include_lib("common_test/include/ct.hrl").
@@ -36,7 +36,8 @@ suite() ->
all() ->
[integer_lists, binary_lists, empty_lists, empty_binary_lists, mixed_lists,
sub_binary_lists, illegal_lists, improper_lists, cons_bomb,
- iolist_to_iovec_idempotence, iolist_to_iovec_correctness].
+ iolist_to_iovec_idempotence, iolist_to_iovec_correctness,
+ unaligned_sub_binaries].
init_per_suite(Config) ->
Config.
@@ -78,7 +79,7 @@ illegal_lists(Config) when is_list(Config) ->
BitStrs = gen_variations(["gurka", <<1:1>>, "gaffel"]),
BadInts = gen_variations(["gurka", 890, "gaffel"]),
Atoms = gen_variations([gurka, "gaffel"]),
- BadTails = [["test" | 0], ["gurka", gaffel]],
+ BadTails = [["test" | 0], ["gurka" | gaffel], ["gaffel" | <<1:1>>]],
Variations =
BitStrs ++ BadInts ++ Atoms ++ BadTails,
@@ -98,14 +99,7 @@ cons_bomb(Config) when is_list(Config) ->
BinBase = gen_variations([<<I:8>> || I <- lists:seq(1, 255)]),
MixBase = gen_variations([<<12, 45, 78>>, lists:seq(1, 255)]),
- Rounds =
- case system_mem_size() of
- Mem when Mem >= (16 bsl 30) -> 32;
- Mem when Mem >= (3 bsl 30) -> 28;
- _ -> 20
- end,
-
- Variations = gen_variations([IntBase, BinBase, MixBase], Rounds),
+ Variations = gen_variations([IntBase, BinBase, MixBase], 16),
equivalence_test(fun erlang:iolist_to_iovec/1, Variations).
iolist_to_iovec_idempotence(Config) when is_list(Config) ->
@@ -130,6 +124,15 @@ iolist_to_iovec_correctness(Config) when is_list(Config) ->
true = is_iolist_equal(Optimized, Variations),
ok.
+unaligned_sub_binaries(Config) when is_list(Config) ->
+ UnalignedBins = [gen_unaligned_binary(I) || I <- lists:seq(32, 4 bsl 10, 512)],
+ UnalignedVariations = gen_variations(UnalignedBins),
+
+ Optimized = erlang:iolist_to_iovec(UnalignedVariations),
+
+ true = is_iolist_equal(Optimized, UnalignedVariations),
+ ok.
+
illegality_test(Fun, Variations) ->
[{'EXIT',{badarg, _}} = (catch Fun(Variation)) || Variation <- Variations],
ok.
@@ -145,11 +148,18 @@ equivalence_test(Fun, [Head | _] = Variations) ->
is_iolist_equal(A, B) ->
iolist_to_binary(A) =:= iolist_to_binary(B).
+gen_unaligned_binary(Size) ->
+ Bin0 = << <<I>> || I <- lists:seq(1, Size) >>,
+ <<0:3,Bin:Size/binary,31:5>> = id(<<0:3,Bin0/binary,31:5>>),
+ Bin.
+
+id(I) -> I.
+
%% Generates a bunch of lists whose contents will be equal to Base repeated a
%% few times. The lists only differ by their structure, so their reduction to
%% a simpler format should yield the same result.
gen_variations(Base) ->
- gen_variations(Base, 16).
+ gen_variations(Base, 12).
gen_variations(Base, N) ->
[gen_flat_list(Base, N),
gen_nested_list(Base, N),
@@ -169,8 +179,3 @@ gen_nasty_list_1([Head | Base], Result) when is_list(Head) ->
gen_nasty_list_1(Base, [[Result], [gen_nasty_list_1(Head, [])]]);
gen_nasty_list_1([Head | Base], Result) ->
gen_nasty_list_1(Base, [[Result], [Head]]).
-
-system_mem_size() ->
- application:ensure_all_started(os_mon),
- {Tot,_Used,_} = memsup:get_memory_data(),
- Tot.
diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index 4811244b98..ac2b136a38 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -2983,8 +2983,24 @@ nif_ioq(Config) ->
{enqv, a, 5, iolist_size(nif_ioq_payload(5)) - 1},
{peek, a},
- %% Test to enqueue a bunch of refc binaries
+ %% Ensure that enqueued refc binaries are intact after a roundtrip.
+ %%
+ %% This test and the ones immediately following it does not go through
+ %% erlang:iolist_to_iovec/1
{enqv, a, [nif_ioq_payload(refcbin) || _ <- lists:seq(1,20)], 0},
+ {peek, a},
+
+ %% ... heap binaries
+ {enqv, a, [nif_ioq_payload(heapbin) || _ <- lists:seq(1,20)], 0},
+ {peek, a},
+
+ %% ... plain sub-binaries
+ {enqv, a, [nif_ioq_payload(subbin) || _ <- lists:seq(1,20)], 0},
+ {peek, a},
+
+ %% ... unaligned binaries
+ {enqv, a, [nif_ioq_payload(unaligned_bin) || _ <- lists:seq(1,20)], 0},
+ {peek, a},
%% Enq stuff to destroy with data in queue
{enqv, a, 2, 100},
@@ -3120,13 +3136,16 @@ nif_ioq_payload(N) when is_integer(N) ->
Tail = if N > 3 -> nif_ioq_payload(N-3); true -> [] end,
Head = element(1, lists:split(N,[nif_ioq_payload(subbin),
nif_ioq_payload(heapbin),
- nif_ioq_payload(refcbin) | Tail])),
+ nif_ioq_payload(refcbin),
+ nif_ioq_payload(unaligned_bin) | Tail])),
erlang:iolist_to_iovec(Head);
nif_ioq_payload(subbin) ->
Bin = nif_ioq_payload(refcbin),
Sz = size(Bin) - 1,
<<_:8,SubBin:Sz/binary,_/bits>> = Bin,
SubBin;
+nif_ioq_payload(unaligned_bin) ->
+ make_unaligned_binary(<< <<I>> || I <- lists:seq(1, 255) >>);
nif_ioq_payload(heapbin) ->
<<"a literal heap binary">>;
nif_ioq_payload(refcbin) ->
@@ -3134,6 +3153,13 @@ nif_ioq_payload(refcbin) ->
nif_ioq_payload(Else) ->
Else.
+make_unaligned_binary(Bin0) ->
+ Size = byte_size(Bin0),
+ <<0:3,Bin:Size/binary,31:5>> = id(<<0:3,Bin0/binary,31:5>>),
+ Bin.
+
+id(I) -> I.
+
%% The NIFs:
lib_version() -> undefined.
call_history() -> ?nif_stub.