hipe: Fix signed compares of unsigned sizes

Also, some of the branches were testing sizes in bits against a constant
?MAX_BINSIZE, which was in bytes. The signed comparisons masked this
mistake. These branches have been removed since all sizes in bits that
fit in a machine word are valid binary sizes.

Finally, a test that reproduces the issue was added to bs_construct,
along with a test for one of the cases (bs_init<0>(...)) when the test
against ?MAX_BINSIZE must be changed to unsigned rather than removed.

4 files changed, 57 insertions, 28 deletions

diff --git a/erts/emulator/test/bs_construct_SUITE.erl b/erts/emulator/test/bs_construct_SUITE.erl
index f2bd6c233a..7ed99f5b4e 100644
--- a/erts/emulator/test/bs_construct_SUITE.erl
+++ b/erts/emulator/test/bs_construct_SUITE.erl
@@ -925,8 +925,8 @@ append_unit_8(Bin) ->
 append_unit_16(Bin) ->
     <<Bin/binary-unit:16,0:1>>.
 
-%% Produce a large result of bs_add that would fit a smallnum if it was viewed
-%% as signed.
+%% Produce a large result of bs_add that, if cast to signed int, would overflow
+%% into a negative number that fits a smallnum.
 bs_add_overflow(Config) ->
     case erlang:system_info(wordsize) of
 	8 ->
diff --git a/lib/hipe/rtl/hipe_rtl_binary_construct.erl b/lib/hipe/rtl/hipe_rtl_binary_construct.erl
index 80436a6931..4403aa552f 100644
--- a/lib/hipe/rtl/hipe_rtl_binary_construct.erl
+++ b/lib/hipe/rtl/hipe_rtl_binary_construct.erl
@@ -386,7 +386,7 @@ not_writable_code(Bin, SizeReg, Dst, Base, Offset, Unit,
    hipe_rtl:mk_alu(TotBytes, TotSize, add, ?LOW_BITS),
    hipe_rtl:mk_alu(TotBytes, TotBytes, srl, ?BYTE_SHIFT),
    hipe_rtl:mk_alu(UsedBytes, TotBytes, sll, hipe_rtl:mk_imm(1)),
-   hipe_rtl:mk_branch(UsedBytes, ge, hipe_rtl:mk_imm(256), 
+   hipe_rtl:mk_branch(UsedBytes, geu, hipe_rtl:mk_imm(256),
 		      AllLblName, IncLblName),
    IncLbl,
    hipe_rtl:mk_move(UsedBytes, hipe_rtl:mk_imm(256)),
@@ -432,7 +432,7 @@ realloc_binary(SizeReg, ProcBin, Base) ->
    set_field_from_term(ProcBinFlagsTag, ProcBin, Flags),
    get_field_from_term(ProcBinValTag, ProcBin, BinPointer),
    get_field_from_pointer(BinOrigSizeTag, BinPointer, OrigSize),
-   hipe_rtl:mk_branch(OrigSize, 'lt', ResultingSize, 
+   hipe_rtl:mk_branch(OrigSize, 'ltu', ResultingSize,
 		      ReallocLblName, NoReallocLblName),
    NoReallocLbl,
    get_field_from_term(ProcBinBytesTag, ProcBin, Base),
@@ -674,13 +674,13 @@ var_init2(Size, Dst, Base, Offset, TrueLblName, SystemLimitLblName, FalseLblName
   [ContLbl,HeapLbl,REFCLbl,NextLbl] = create_lbls(4),
   [USize,Tmp] = create_unsafe_regs(2),
   [get_word_integer(Size, USize, SystemLimitLblName, FalseLblName),
-   hipe_rtl:mk_branch(USize, le, hipe_rtl:mk_imm(?MAX_BINSIZE), 
-		      hipe_rtl:label_name(ContLbl), 
-		      SystemLimitLblName),
+   hipe_rtl:mk_branch(USize, leu, hipe_rtl:mk_imm(?MAX_BINSIZE),
+                     hipe_rtl:label_name(ContLbl),
+                     SystemLimitLblName),
    ContLbl,
    hipe_rtl:mk_move(Offset, hipe_rtl:mk_imm(0)),
-   hipe_rtl:mk_branch(USize, le, hipe_rtl:mk_imm(?MAX_HEAP_BIN_SIZE), 
-		      hipe_rtl:label_name(HeapLbl), 
+   hipe_rtl:mk_branch(USize, leu, hipe_rtl:mk_imm(?MAX_HEAP_BIN_SIZE),
+		      hipe_rtl:label_name(HeapLbl),
 		      hipe_rtl:label_name(REFCLbl)),
    HeapLbl,
    hipe_rtl:mk_alu(Tmp, USize, add, hipe_rtl:mk_imm(3*WordSize-1)),
@@ -698,8 +698,8 @@ var_init2(Size, Dst, Base, Offset, TrueLblName, SystemLimitLblName, FalseLblName
    hipe_rtl:mk_goto(TrueLblName)].
 
 var_init_bits(Size, Dst, Base, Offset, TrueLblName, SystemLimitLblName, FalseLblName) ->
-  [HeapLbl,REFCLbl,NextLbl,NoSubLbl,SubLbl,ContLbl,
-   NoCreateSubBin, CreateSubBin, JoinLbl, JoinLbl2] = create_lbls(10),
+  [HeapLbl,REFCLbl,NextLbl,NoSubLbl,SubLbl,
+   NoCreateSubBin, CreateSubBin, JoinLbl, JoinLbl2] = create_lbls(9),
   [USize,ByteSize,TotByteSize,OffsetBits] = create_regs(4),
   [TmpDst] = create_unsafe_regs(1),
   Log2WordSize = hipe_rtl_arch:log2_word_size(),
@@ -720,11 +720,7 @@ var_init_bits(Size, Dst, Base, Offset, TrueLblName, SystemLimitLblName, FalseLbl
    SubLbl,
    hipe_rtl:mk_alu(TotByteSize, ByteSize, 'add', hipe_rtl:mk_imm(1)),
    JoinLbl,
-   hipe_rtl:mk_branch(USize, le, hipe_rtl:mk_imm(?MAX_BINSIZE), 
-		      hipe_rtl:label_name(ContLbl), 
-		      SystemLimitLblName),
-   ContLbl,
-   hipe_rtl:mk_branch(TotByteSize, 'le', hipe_rtl:mk_imm(?MAX_HEAP_BIN_SIZE),
+   hipe_rtl:mk_branch(TotByteSize, 'leu', hipe_rtl:mk_imm(?MAX_HEAP_BIN_SIZE),
 		      hipe_rtl:label_name(HeapLbl), 
 		      hipe_rtl:label_name(REFCLbl)),
    HeapLbl,
@@ -983,7 +979,7 @@ copy_string(StringBase, StringSize, BinBase, BinOffset, NewOffset, TrueLblName)
 
 small_check(SizeVar, CopySize, FalseLblName) ->
   SuccessLbl = hipe_rtl:mk_new_label(),
-  [hipe_rtl:mk_branch(SizeVar, le, CopySize,
+  [hipe_rtl:mk_branch(SizeVar, leu, CopySize,
 		      hipe_rtl:label_name(SuccessLbl), FalseLblName),
    SuccessLbl].
 
diff --git a/lib/hipe/rtl/hipe_rtl_binary_match.erl b/lib/hipe/rtl/hipe_rtl_binary_match.erl
index b385ea6afc..b7c4dc2536 100644
--- a/lib/hipe/rtl/hipe_rtl_binary_match.erl
+++ b/lib/hipe/rtl/hipe_rtl_binary_match.erl
@@ -37,7 +37,6 @@
 
 %%--------------------------------------------------------------------
 
--define(MAX_BINSIZE, trunc(?MAX_HEAP_BIN_SIZE / hipe_rtl_arch:word_size()) + 2).
 -define(BYTE_SHIFT, 3). %% Turn bits into bytes or vice versa
 -define(LOW_BITS, 7). %% Three lowest bits set 
 -define(BYTE_SIZE, 8).
@@ -819,10 +818,10 @@ create_lbls(0) ->
 create_lbls(X) when X > 0 ->
   [hipe_rtl:mk_new_label()|create_lbls(X-1)].
 
-make_dyn_prep(SizeReg, CCode) ->   
+make_dyn_prep(SizeReg, CCode) ->
   [CLbl, SuccessLbl] = create_lbls(2),
-  Init = [hipe_rtl:mk_branch(SizeReg, le, hipe_rtl:mk_imm(?MAX_SMALL_BITS),  
-			     hipe_rtl:label_name(SuccessLbl), 
+  Init = [hipe_rtl:mk_branch(SizeReg, leu, hipe_rtl:mk_imm(?MAX_SMALL_BITS),
+			     hipe_rtl:label_name(SuccessLbl),
 			     hipe_rtl:label_name(CLbl)),
 	  SuccessLbl],
   End = [CLbl|CCode],
@@ -867,8 +866,8 @@ get_unaligned_int_to_reg(Reg, Size, Base, Offset, LowBits, Shiftr, Type) ->
 			 hipe_rtl:mk_imm((WordSize-MinLoad)*?BYTE_SIZE))];
       {_, WordSize} ->
 	UnsignedBig = {unsigned, big},
-	[hipe_rtl:mk_branch(TotBits, le, hipe_rtl:mk_imm(MinLoad*?BYTE_SIZE), 
-			    hipe_rtl:label_name(LessLbl), 
+	[hipe_rtl:mk_branch(TotBits, leu, hipe_rtl:mk_imm(MinLoad*?BYTE_SIZE),
+			    hipe_rtl:label_name(LessLbl),
 			    hipe_rtl:label_name(MoreLbl)),
 	 LessLbl,
 	 load_bytes(LoadDst, Base, ByteOffset, Type, MinLoad),
@@ -928,7 +927,7 @@ get_big_unknown_int(Dst1, Base, Offset, NewOffset,
    hipe_rtl:mk_alu(ByteOffset, Offset, srl, hipe_rtl:mk_imm(?BYTE_SHIFT)),
    load_bytes(LoadDst, Base, ByteOffset, Type, 1),
    BackLbl,
-   hipe_rtl:mk_branch(ByteOffset, le, Limit, hipe_rtl:label_name(LoopLbl), 
+   hipe_rtl:mk_branch(ByteOffset, leu, Limit, hipe_rtl:label_name(LoopLbl),
 		      hipe_rtl:label_name(EndLbl)),
    LoopLbl,
    load_bytes(Tmp, Base, ByteOffset, {unsigned, big}, 1),
@@ -957,8 +956,8 @@ get_little_unknown_int(Dst1, Base, Offset, NewOffset,
    hipe_rtl:mk_alu(Limit, Tmp, srl, hipe_rtl:mk_imm(?BYTE_SHIFT)),
    hipe_rtl:mk_move(ShiftReg, hipe_rtl:mk_imm(0)),
    BackLbl,
-   hipe_rtl:mk_branch(ByteOffset, lt, Limit, 
-		      hipe_rtl:label_name(LoopLbl), 
+   hipe_rtl:mk_branch(ByteOffset, ltu, Limit,
+		      hipe_rtl:label_name(LoopLbl),
 		      hipe_rtl:label_name(DoneLbl)),
    LoopLbl,
    load_bytes(Tmp, Base, ByteOffset, {unsigned, big}, 1),
@@ -1074,7 +1073,7 @@ load_bytes(Dst, Base, Offset, {Signedness, Endianness}, X) when X > 1 ->
        hipe_rtl:mk_alu(Offset, Offset, add, hipe_rtl:mk_imm(1)),
        hipe_rtl:mk_alu(Dst, Dst, sll, hipe_rtl:mk_imm(8)),
        hipe_rtl:mk_alu(Dst, Dst, 'or', Tmp1),
-       hipe_rtl:mk_branch(Offset, lt, Limit, hipe_rtl:label_name(LoopLbl),
+       hipe_rtl:mk_branch(Offset, ltu, Limit, hipe_rtl:label_name(LoopLbl),
 			  hipe_rtl:label_name(EndLbl)),
        EndLbl];
     little ->
@@ -1086,7 +1085,7 @@ load_bytes(Dst, Base, Offset, {Signedness, Endianness}, X) when X > 1 ->
        hipe_rtl:mk_load(Tmp1, Base, TmpOffset, byte, Signedness),
        hipe_rtl:mk_alu(Dst, Dst, sll, hipe_rtl:mk_imm(8)),
        hipe_rtl:mk_alu(Dst, Dst, 'or', Tmp1),
-       hipe_rtl:mk_branch(Offset, lt, TmpOffset, hipe_rtl:label_name(LoopLbl),
+       hipe_rtl:mk_branch(Offset, ltu, TmpOffset, hipe_rtl:label_name(LoopLbl),
 			  hipe_rtl:label_name(EndLbl)),
        EndLbl,
        hipe_rtl:mk_move(Offset, Limit)]
diff --git a/lib/hipe/test/bs_SUITE_data/bs_construct.erl b/lib/hipe/test/bs_SUITE_data/bs_construct.erl
index 7251cdce43..b9e7d93570 100644
--- a/lib/hipe/test/bs_SUITE_data/bs_construct.erl
+++ b/lib/hipe/test/bs_SUITE_data/bs_construct.erl
@@ -18,6 +18,7 @@ test() ->
   ok = bad_append(),
   ok = system_limit(),
   ok = bad_floats(),
+  ok = huge_binaries(),
   ok.
 
 %%--------------------------------------------------------------------
@@ -269,3 +270,36 @@ bad_floats() ->
   {'EXIT',{badarg,_}} = (catch <<3.14:(id((1 bsl 28) bor 32))/float>>),
   {'EXIT',{system_limit,_}} = (catch <<3.14:(id(1 bsl BitsPerWord))/float>>),
   ok.
+
+%%--------------------------------------------------------------------
+%% A bug in the implementation of binaries compared sizes in bits with sizes in
+%% bytes, causing <<0:(id((1 bsl 31)-1))>> to fail to construct with
+%% 'system_limit'.
+%% <<0:(id((1 bsl 32)-1))>> was succeeding because the comparison was
+%% (incorrectly) signed.
+
+huge_binaries() ->
+  AlmostIllegal = id(<<0:(id((1 bsl 32)-8))>>),
+  case erlang:system_info(wordsize) of
+    4 -> huge_binaries_32(AlmostIllegal);
+    8 -> ok
+  end,
+  garbage_collect(),
+  id(<<0:(id((1 bsl 31)-1))>>),
+  id(<<0:(id((1 bsl 30)-1))>>),
+  garbage_collect(),
+  ok.
+
+huge_binaries_32(AlmostIllegal) ->
+  %% Attempt construction of too large binary using bs_init/1 (which takes the
+  %% number of bytes as an argument, which should be compared to the maximum
+  %% size in bytes).
+  {'EXIT',{system_limit,_}} = (catch <<0:32,AlmostIllegal/binary>>),
+  %% Attempt construction of too large binary using bs_init/1 with a size in
+  %% bytes that has the msb set (and would be negative if it was signed).
+  {'EXIT',{system_limit,_}} =
+    (catch <<0:8, AlmostIllegal/binary, AlmostIllegal/binary,
+	     AlmostIllegal/binary, AlmostIllegal/binary,
+	     AlmostIllegal/binary, AlmostIllegal/binary,
+	     AlmostIllegal/binary, AlmostIllegal/binary>>),
+  ok.