Fix binary append exceptions

Code such as the following should cause an exception:

  Bin = <<0:1>>,
  <<Bin/binary,...>>

(Because the "binary" type implies that the binary must have a
size in bits that is a multiple of 8.)

Depending on the binary, either no exception was thrown or the
wrong exception was thrown (because c_p->freason was not set).

Noticed-by: Adam Rutkowski

3 files changed, 101 insertions, 9 deletions

diff --git a/erts/emulator/beam/erl_bits.c b/erts/emulator/beam/erl_bits.c
index 6bc227eeda..3753b618e1 100644
--- a/erts/emulator/beam/erl_bits.c
+++ b/erts/emulator/beam/erl_bits.c
@@ -1247,6 +1247,12 @@ erts_bs_append(Process* c_p, Eterm* reg, Uint live, Eterm build_size_term,
      */
 
     erts_bin_offset = 8*sb->size + sb->bitsize;
+    if (unit > 1) {
+	if ((unit == 8 && (erts_bin_offset & 7) != 0) ||
+	    (erts_bin_offset % unit) != 0) {
+	    goto badarg;
+	}
+    }
     used_size_in_bits = erts_bin_offset + build_size_in_bits;
     sb->is_writable = 0;	/* Make sure that no one else can write. */
     pb->size = NBYTES(used_size_in_bits);
@@ -1316,6 +1322,12 @@ erts_bs_append(Process* c_p, Eterm* reg, Uint live, Eterm build_size_term,
 	 */
 	ERTS_GET_BINARY_BYTES(bin, src_bytes, bitoffs, bitsize);
 	erts_bin_offset = 8*binary_size(bin) + bitsize;
+	if (unit > 1) {
+	    if ((unit == 8 && (erts_bin_offset & 7) != 0) ||
+		(erts_bin_offset % unit) != 0) {
+		goto badarg;
+	    }
+	}
 	used_size_in_bits = erts_bin_offset + build_size_in_bits;
 	used_size_in_bytes = NBYTES(used_size_in_bits);
 	bin_size = 2*used_size_in_bytes;
@@ -1363,12 +1375,6 @@ erts_bs_append(Process* c_p, Eterm* reg, Uint live, Eterm build_size_term,
 	/*
 	 * Now copy the data into the binary.
 	 */
-	if (unit > 1) {
-	    if ((unit == 8 && (erts_bin_offset & 7) != 0) ||
-		(erts_bin_offset % unit) != 0) {
-		return THE_NON_VALUE;
-	    }
-	}
 	copy_binary_to_buffer(erts_current_bin, 0, src_bytes, bitoffs, erts_bin_offset);
 
 	return make_binary(sb);
diff --git a/erts/emulator/test/bs_bit_binaries_SUITE.erl b/erts/emulator/test/bs_bit_binaries_SUITE.erl
index ff1088118d..1428387a65 100644
--- a/erts/emulator/test/bs_bit_binaries_SUITE.erl
+++ b/erts/emulator/test/bs_bit_binaries_SUITE.erl
@@ -177,14 +177,55 @@ append(Config) when is_list(Config) ->
     cs_init(),
     ?line <<(-1):256/signed-unit:8>> = cs(do_append(id(<<>>), 256*8)),
     ?line <<(-1):256/signed-unit:8>> = cs(do_append2(id(<<>>), 256*4)),
+    <<(-1):256/signed-unit:8>> = cs(do_append3(id(<<>>), 256*8)),
     cs_end().
     
 do_append(Bin, N) when N > 0 -> do_append(<<Bin/bits,1:1>>, N-1);
 do_append(Bin, 0) -> Bin.
 
-do_append2(Bin, N) when N > 0 -> do_append2(<<Bin/bits,3:2>>, N-1);
+do_append2(Bin, N) when N > 0 -> do_append2(<<Bin/binary-unit:2,3:2>>, N-1);
 do_append2(Bin, 0) -> Bin.
 
+do_append3(Bin, N) when N > 0 ->
+    Bits = bit_size(Bin),
+    if
+	Bits rem 2 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:2,1:1>>, N-1);
+	Bits rem 3 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:3,1:1>>, N-1);
+	Bits rem 4 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:4,1:1>>, N-1);
+	Bits rem 5 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:5,1:1>>, N-1);
+	Bits rem 6 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:6,1:1>>, N-1);
+	Bits rem 7 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:7,1:1>>, N-1);
+	Bits rem 8 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:8,1:1>>, N-1);
+	Bits rem 9 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:9,1:1>>, N-1);
+	Bits rem 10 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:10,1:1>>, N-1);
+	Bits rem 11 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:11,1:1>>, N-1);
+	Bits rem 12 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:12,1:1>>, N-1);
+	Bits rem 13 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:13,1:1>>, N-1);
+	Bits rem 14 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:14,1:1>>, N-1);
+	Bits rem 15 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:15,1:1>>, N-1);
+	Bits rem 16 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:16,1:1>>, N-1);
+	Bits rem 17 =:= 0 ->
+	    do_append3(<<Bin/binary-unit:17,1:1>>, N-1);
+	true ->
+	    do_append3(<<Bin/binary-unit:1,1:1>>, N-1)
+    end;
+do_append3(Bin, 0) -> Bin.
+
 cs_init() ->
     erts_debug:set_internal_state(available_internal_state, true),
     ok.
diff --git a/erts/emulator/test/bs_construct_SUITE.erl b/erts/emulator/test/bs_construct_SUITE.erl
index 8b5c82d968..9c88803fea 100644
--- a/erts/emulator/test/bs_construct_SUITE.erl
+++ b/erts/emulator/test/bs_construct_SUITE.erl
@@ -28,7 +28,7 @@
 	 mem_leak/1, coerce_to_float/1, bjorn/1,
 	 huge_float_field/1, huge_binary/1, system_limit/1, badarg/1,
 	 copy_writable_binary/1, kostis/1, dynamic/1, bs_add/1,
-	 otp_7422/1, zero_width/1]).
+	 otp_7422/1, zero_width/1, bad_append/1]).
 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -38,7 +38,8 @@ all() ->
     [test1, test2, test3, test4, test5, testf, not_used,
      in_guard, mem_leak, coerce_to_float, bjorn,
      huge_float_field, huge_binary, system_limit, badarg,
-     copy_writable_binary, kostis, dynamic, bs_add, otp_7422, zero_width].
+     copy_writable_binary, kostis, dynamic, bs_add, otp_7422, zero_width,
+     bad_append].
 
 groups() -> 
     [].
@@ -827,5 +828,49 @@ zero_width(Config) when is_list(Config) ->
     ?line {'EXIT',{badarg,_}} = (catch <<(id(not_a_number)):0>>),
 
     ok.
+
+bad_append(_) ->
+    do_bad_append(<<127:1>>, fun append_unit_3/1),
+    do_bad_append(<<127:2>>, fun append_unit_3/1),
+    do_bad_append(<<127:17>>, fun append_unit_3/1),
+
+    do_bad_append(<<127:3>>, fun append_unit_4/1),
+    do_bad_append(<<127:5>>, fun append_unit_4/1),
+    do_bad_append(<<127:7>>, fun append_unit_4/1),
+    do_bad_append(<<127:199>>, fun append_unit_4/1),
+
+    do_bad_append(<<127:7>>, fun append_unit_8/1),
+    do_bad_append(<<127:9>>, fun append_unit_8/1),
+
+    do_bad_append(<<0:8>>, fun append_unit_16/1),
+    do_bad_append(<<0:15>>, fun append_unit_16/1),
+    do_bad_append(<<0:17>>, fun append_unit_16/1),
+    ok.
+
+do_bad_append(Bin0, Appender) ->
+    {'EXIT',{badarg,_}} = (catch Appender(Bin0)),
+
+    Bin1 = id(<<0:3,Bin0/bitstring>>),
+    <<_:3,Bin2/bitstring>> = Bin1,
+    {'EXIT',{badarg,_}} = (catch Appender(Bin2)),
+
+    %% Create a writable binary.
+    Empty = id(<<>>),
+    Bin3 = <<Empty/bitstring,Bin0/bitstring>>,
+    {'EXIT',{badarg,_}} = (catch Appender(Bin3)),
+    ok.
+
+append_unit_3(Bin) ->
+    <<Bin/binary-unit:3,0:1>>.
+
+append_unit_4(Bin) ->
+    <<Bin/binary-unit:4,0:1>>.
+
+append_unit_8(Bin) ->
+    <<Bin/binary,0:1>>.
+
+append_unit_16(Bin) ->
+    <<Bin/binary-unit:16,0:1>>.
+
     
 id(I) -> I.