Merge branch 'bjorn/erts/loader-related-bs-bugs/OTP-9284' into dev

* bjorn/erts/loader-related-bs-bugs/OTP-9284:
  Fix binary construction with huge literal sizes
  beam_load.c: Add overflow check of tag values
  beam_makeops: Add some sanity checks
  Fix construction of <<0:((1 bsl 32)-1)>>

6 files changed, 150 insertions, 125 deletions

diff --git a/erts/emulator/beam/beam_emu.c b/erts/emulator/beam/beam_emu.c
index fb90a7d4f7..937b3d9e53 100644
--- a/erts/emulator/beam/beam_emu.c
+++ b/erts/emulator/beam/beam_emu.c
@@ -3561,7 +3561,7 @@ void process_main(void)
       * Operands: NotUsed Live Dst
       */
  do_bs_init_bits_known:
-     num_bytes = (num_bits+7) >> 3;
+     num_bytes = ((Uint64)num_bits+(Uint64)7) >> 3;
      if (num_bits & 7) {
 	 alloc += ERL_SUB_BIN_SIZE;
      }
diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 57fe25453d..eb10ae59a8 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -332,20 +332,22 @@ typedef struct {
     Eterm* func_tab[1];		/* Pointers to each function. */
 } LoadedCode;
 
-#define GetTagAndValue(Stp, Tag, Val) \
-   do { \
-      BeamInstr __w; \
-      GetByte(Stp, __w); \
-      Tag = __w & 0x07; \
-      if ((__w & 0x08) == 0) { \
-	 Val = __w >> 4; \
-      } else if ((__w & 0x10) == 0) { \
-	 Val = ((__w >> 5) << 8); \
-	 GetByte(Stp, __w); \
-	 Val |= __w; \
-      } else { \
-	if (!get_int_val(Stp, __w, &(Val))) goto load_error; \
-      } \
+#define GetTagAndValue(Stp, Tag, Val)					\
+   do {									\
+      BeamInstr __w;							\
+      GetByte(Stp, __w);						\
+      Tag = __w & 0x07;							\
+      if ((__w & 0x08) == 0) {						\
+	 Val = __w >> 4;						\
+      } else if ((__w & 0x10) == 0) {					\
+	 Val = ((__w >> 5) << 8);					\
+	 GetByte(Stp, __w);						\
+	 Val |= __w;							\
+      } else {								\
+	 int __res = get_tag_and_value(Stp, __w, (Tag), &(Val));	\
+         if (__res < 0) goto load_error;				\
+         Tag = (unsigned) __res;					\
+      }									\
    } while (0)
 
 
@@ -489,8 +491,8 @@ static void load_printf(int line, LoaderState* context, char *fmt, ...);
 static int transform_engine(LoaderState* st);
 static void id_to_string(Uint id, char* s);
 static void new_genop(LoaderState* stp);
-static int get_int_val(LoaderState* stp, Uint len_code, BeamInstr* result);
-static int get_erlang_integer(LoaderState* stp, Uint len_code, BeamInstr* result);
+static int get_tag_and_value(LoaderState* stp, Uint len_code,
+			     unsigned tag, BeamInstr* result);
 static int new_label(LoaderState* stp);
 static void new_literal_patch(LoaderState* stp, int pos);
 static void new_string_patch(LoaderState* stp, int pos);
@@ -1470,46 +1472,15 @@ load_code(LoaderState* stp)
 	last_op->arity = 0;
 	ASSERT(arity <= MAX_OPARGS);
 
-#define GetValue(Stp, First, Val) \
-   do { \
-      if (((First) & 0x08) == 0) { \
-	 Val = (First) >> 4; \
-      } else if (((First) & 0x10) == 0) { \
-         BeamInstr __w; \
-	 GetByte(Stp, __w); \
-	 Val = (((First) >> 5) << 8) | __w; \
-      } else { \
-	if (!get_int_val(Stp, (First), &(Val))) goto load_error; \
-      } \
-   } while (0)
-
 	for (arg = 0; arg < arity; arg++) {
-	    BeamInstr first;
-
-	    GetByte(stp, first);
-	    last_op->a[arg].type = first & 0x07;
+	    GetTagAndValue(stp, last_op->a[arg].type, last_op->a[arg].val);
 	    switch (last_op->a[arg].type) {
 	    case TAG_i:
-		if ((first & 0x08) == 0) {
-		    last_op->a[arg].val = first >> 4;
-		} else if ((first & 0x10) == 0) {
-		    BeamInstr w;
-		    GetByte(stp, w);
-		    ASSERT(first < 0x800);
-		    last_op->a[arg].val = ((first >> 5) << 8) | w;
-		} else {
-		    int i = get_erlang_integer(stp, first, &(last_op->a[arg].val));
-		    if (i < 0) {
-			goto load_error;
-		    }
-		    last_op->a[arg].type = i;
-		}
-		break;
 	    case TAG_u:
-		GetValue(stp, first, last_op->a[arg].val);
+	    case TAG_q:
+	    case TAG_o:
 		break;
 	    case TAG_x:
-		GetValue(stp, first, last_op->a[arg].val);
 		if (last_op->a[arg].val == 0) {
 		    last_op->a[arg].type = TAG_r;
 		} else if (last_op->a[arg].val >= MAX_REG) {
@@ -1518,7 +1489,6 @@ load_code(LoaderState* stp)
 		}
 		break;
 	    case TAG_y:
-		GetValue(stp, first, last_op->a[arg].val);
 		if (last_op->a[arg].val >= MAX_REG) {
 		    LoadError1(stp, "invalid y register number: %u",
 			       last_op->a[arg].val);
@@ -1526,7 +1496,6 @@ load_code(LoaderState* stp)
 		last_op->a[arg].val += CP_SIZE;
 		break;
 	    case TAG_a:
-		GetValue(stp, first, last_op->a[arg].val);
 		if (last_op->a[arg].val == 0) {
 		    last_op->a[arg].type = TAG_n;
 		} else if (last_op->a[arg].val >= stp->num_atoms) {
@@ -1536,7 +1505,6 @@ load_code(LoaderState* stp)
 		}
 		break;
 	    case TAG_f:
-		GetValue(stp, first, last_op->a[arg].val);
 		if (last_op->a[arg].val == 0) {
 		    last_op->a[arg].type = TAG_p;
 		} else if (last_op->a[arg].val >= stp->num_labels) {
@@ -1544,7 +1512,6 @@ load_code(LoaderState* stp)
 		}
 		break;
 	    case TAG_h:
-		GetValue(stp, first, last_op->a[arg].val);
 		if (last_op->a[arg].val > 65535) {
 		    LoadError1(stp, "invalid range for character data type: %u",
 			       last_op->a[arg].val);
@@ -1552,11 +1519,9 @@ load_code(LoaderState* stp)
 		break;
 	    case TAG_z:
 		{
-		    BeamInstr ext_tag;
 		    unsigned tag;
 
-		    GetValue(stp, first, ext_tag);
-		    switch (ext_tag) {
+		    switch (last_op->a[arg].val) {
 		    case 0:	/* Floating point number */
 			{
 			    Eterm* hp;
@@ -1648,7 +1613,8 @@ load_code(LoaderState* stp)
 			    break;
 			}
 		    default:
-			LoadError1(stp, "invalid extended tag %d", ext_tag);
+			LoadError1(stp, "invalid extended tag %d",
+				   last_op->a[arg].val);
 			break;
 		    }
 		}
@@ -1659,7 +1625,6 @@ load_code(LoaderState* stp)
 	    }
 	    last_op->arity++;
 	}
-#undef GetValue
 
 	ASSERT(arity == last_op->arity);
 
@@ -2562,13 +2527,8 @@ should_gen_heap_bin(LoaderState* stp, GenOpArg Src)
 static int
 binary_too_big(LoaderState* stp, GenOpArg Size)
 {
-    return Size.type == TAG_u && ((Size.val >> (8*sizeof(Uint)-3)) != 0);
-}
-
-static int
-binary_too_big_bits(LoaderState* stp, GenOpArg Size)
-{
-    return Size.type == TAG_u && (((Size.val+7)/8) >> (8*sizeof(Uint)-3) != 0);
+    return Size.type == TAG_o ||
+	(Size.type == TAG_u && ((Size.val >> (8*sizeof(Uint)-3)) != 0));
 }
 
 static GenOp*
@@ -4317,41 +4277,9 @@ load_printf(int line, LoaderState* context, char *fmt,...)
     erts_send_error_to_logger(context->group_leader, dsbufp);
 }
 
-
-static int
-get_int_val(LoaderState* stp, Uint len_code, BeamInstr* result)
-{
-    Uint count;
-    Uint val;
-
-    len_code >>= 5;
-    ASSERT(len_code < 8);
-    if (len_code == 7) {
-	LoadError0(stp, "can't load integers bigger than 8 bytes yet\n");
-    }
-    count = len_code + 2;
-    if (count == 5) {
-	Uint msb;
-	GetByte(stp, msb);
-	if (msb == 0) {
-	    count--;
-	}
-	GetInt(stp, 4, *result);
-    } else if (count <= 4) {
-	GetInt(stp, count, val);
-	*result = ((val << 8*(sizeof(val)-count)) >> 8*(sizeof(val)-count));
-    } else {
-	LoadError1(stp, "too big integer; %d bytes\n", count);
-    }
-    return 1;
-
- load_error:
-    return 0;
-}
-
-
 static int
-get_erlang_integer(LoaderState* stp, Uint len_code, BeamInstr* result)
+get_tag_and_value(LoaderState* stp, Uint len_code,
+		  unsigned tag, BeamInstr* result)
 {
     Uint count;
     Sint val;
@@ -4371,17 +4299,62 @@ get_erlang_integer(LoaderState* stp, Uint len_code, BeamInstr* result)
     if (len_code < 7) {
 	count = len_code + 2;
     } else {
-	Uint tag;
+	unsigned sztag;
 	UWord len_word;
 
 	ASSERT(len_code == 7);
-	GetTagAndValue(stp, tag, len_word);
-	VerifyTag(stp, TAG_u, tag);
+	GetTagAndValue(stp, sztag, len_word);
+	VerifyTag(stp, sztag, TAG_u);
 	count = len_word + 9;
     }
 
     /*
-     * Handle values up to the size of an int, meaning either a small or bignum.
+     * The value for tags except TAG_i must be an unsigned integer
+     * fitting in an Uint. If it does not fit, we'll indicate overflow
+     * by changing the tag to TAG_o.
+     */
+
+    if (tag != TAG_i) {
+	if (count == sizeof(Uint)+1) {
+	    Uint msb;
+
+	    /*
+	     * The encoded value has one more byte than an Uint.
+	     * It will still fit in an Uint if the most significant
+	     * byte is 0.
+	     */
+	    GetByte(stp, msb);
+	    GetInt(stp, sizeof(Uint), *result);
+	    if (msb != 0) {
+		/* Overflow: Negative or too big. */
+		return TAG_o;
+	    }
+	} else if (count == sizeof(Uint)) {
+	    /*
+	     * The value must be positive (or the encoded value would
+	     * have been one byte longer).
+	     */
+	    GetInt(stp, count, *result);
+	} else if (count < sizeof(Uint)) {
+	    GetInt(stp, count, *result);
+
+	    /*
+	     * If the sign bit is set, the value is negative
+	     * (not allowed).
+	     */
+	    if (*result & ((Uint)1 << (count*8-1))) {
+		return TAG_o;
+	    }
+	} else {
+	    GetInt(stp, count, *result);
+	    return TAG_o;
+	}
+	return tag;
+    }
+
+    /*
+     * TAG_i: First handle values up to the size of an Uint (i.e. either
+     * a small or a bignum).
      */
 
     if (count <= sizeof(val)) {
diff --git a/erts/emulator/beam/erl_bits.h b/erts/emulator/beam/erl_bits.h
index 0f67733fa4..3309ea706b 100644
--- a/erts/emulator/beam/erl_bits.h
+++ b/erts/emulator/beam/erl_bits.h
@@ -150,7 +150,7 @@ void erts_bits_destroy_state(ERL_BITS_PROTO_0);
  * NBYTES(x) returns the number of bytes needed to store x bits.
  */
 
-#define NBYTES(x)  (((x) + 7) >> 3) 
+#define NBYTES(x)  (((Uint64)(x) + (Uint64) 7) >> 3) 
 #define BYTE_OFFSET(ofs) ((Uint) (ofs) >> 3)
 #define BIT_OFFSET(ofs) ((ofs) & 7)
 
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 8a5763b4bb..304ce22ef2 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -1236,7 +1236,7 @@ i_bs_init_heap I I I d
 i_bs_init_heap_bin_heap I I I d
 
 
-bs_init_bits Fail Sz Words Regs Flags Dst | binary_too_big_bits(Sz) => system_limit Fail
+bs_init_bits Fail Sz=o Words Regs Flags Dst => system_limit Fail
 
 bs_init_bits Fail Sz=u Words=u==0 Regs Flags Dst => i_bs_init_bits Sz Regs Dst
 bs_init_bits Fail Sz=u Words Regs Flags Dst =>  i_bs_init_bits_heap Sz Words Regs Dst
diff --git a/erts/emulator/test/bs_construct_SUITE.erl b/erts/emulator/test/bs_construct_SUITE.erl
index 1959803385..7fdf36711b 100644
--- a/erts/emulator/test/bs_construct_SUITE.erl
+++ b/erts/emulator/test/bs_construct_SUITE.erl
@@ -553,6 +553,11 @@ huge_float_check({'EXIT',{badarg,_}}) -> ok.
 
 huge_binary(Config) when is_list(Config) ->
     ?line 16777216 = size(<<0:(id(1 bsl 26)),(-1):(id(1 bsl 26))>>),
+    ?line garbage_collect(),
+    ?line id(<<0:((1 bsl 32)-1)>>),
+    ?line garbage_collect(),
+    ?line id(<<0:(id((1 bsl 32)-1))>>),
+    ?line garbage_collect(),
     ok.
 
 system_limit(Config) when is_list(Config) ->
@@ -565,6 +570,10 @@ system_limit(Config) when is_list(Config) ->
     ?line {'EXIT',{system_limit,_}} =
 	(catch <<(id(<<>>))/binary,0:(id(1 bsl 100))>>),
 
+    %% Would fail to load.
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:(1 bsl 67)>>),
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:((1 bsl 64)+1)>>),
+
     case WordSize of
 	4 ->
 	    system_limit_32();
@@ -581,6 +590,14 @@ system_limit_32() ->
     ?line {'EXIT',{system_limit,_}} = (catch <<0:(id(8)),42:536870912/unit:8>>),
     ?line {'EXIT',{system_limit,_}} =
 	(catch <<0:(id(8)),42:(id(536870912))/unit:8>>),
+
+    %% The size would be silently truncated, resulting in a crash.
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:(1 bsl 35)>>),
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:((1 bsl 32)+1)>>),
+
+    %% Would fail to load.
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:(1 bsl 43)>>),
+    ?line {'EXIT',{system_limit,_}} = (catch <<0:((1 bsl 40)+1)>>),
     ok.
 
 badarg(Config) when is_list(Config) ->
diff --git a/erts/emulator/utils/beam_makeops b/erts/emulator/utils/beam_makeops
index e7c57142c0..354439b5e3 100755
--- a/erts/emulator/utils/beam_makeops
+++ b/erts/emulator/utils/beam_makeops
@@ -67,6 +67,10 @@ my $max_gen_operands = 8;
 # Must be even. The beam_load.c file must be updated, too.
 my $max_spec_operands = 6;
 
+# The maximum number of primitive genop_types.
+
+my $max_genop_types = 16;
+
 my %gen_opnum;
 my %num_specific;
 my %gen_to_spec;
@@ -106,7 +110,7 @@ my @pred_table;
 # Operand types for generic instructions.
 
 my $compiler_types = "uiaxyfhz";
-my $loader_types = "nprvlq";
+my $loader_types = "nprvlqo";
 my $genop_types = $compiler_types . $loader_types;
 
 #
@@ -142,34 +146,61 @@ my %arg_size = ('r' => 0,	# x(0) - x register zero
 my %type_bit;
 my @tag_type;
 
+sub define_type_bit {
+    my($tag,$val) = @_;
+    defined $type_bit{$tag} and
+	sanity("the tag '$tag' has already been defined with the value ",
+	       $type_bit{$tag});
+    $type_bit{$tag} = $val;
+}
+
 {
     my($bit) = 1;
     my(%bit);
 
     foreach (split('', $genop_types)) {
 	push(@tag_type, $_);
-	$type_bit{$_} = $bit;
+	define_type_bit($_, $bit);
 	$bit{$_} = $bit;
 	$bit *= 2;
     }
 
     # Composed types.
-    $type_bit{'d'} = $type_bit{'x'} | $type_bit{'y'} | $type_bit{'r'};
-    $type_bit{'c'} = $type_bit{'i'} | $type_bit{'a'} | $type_bit{'n'} | $type_bit{'q'};
-    $type_bit{'s'} = $type_bit{'d'} | $type_bit{'i'} | $type_bit{'a'} |	$type_bit{'n'};
-    $type_bit{'j'} = $type_bit{'f'} | $type_bit{'p'};
+    define_type_bit('d', $type_bit{'x'} | $type_bit{'y'} | $type_bit{'r'});
+    define_type_bit('c', $type_bit{'i'} | $type_bit{'a'} |
+		    $type_bit{'n'} | $type_bit{'q'});
+    define_type_bit('s', $type_bit{'d'} | $type_bit{'i'} |
+		    $type_bit{'a'} | $type_bit{'n'});
+    define_type_bit('j', $type_bit{'f'} | $type_bit{'p'});
 
     # Aliases (for matching purposes).
-    $type_bit{'I'} = $type_bit{'u'};
-    $type_bit{'t'} = $type_bit{'u'};
-    $type_bit{'A'} = $type_bit{'u'};
-    $type_bit{'L'} = $type_bit{'u'};
-    $type_bit{'b'} = $type_bit{'u'};
-    $type_bit{'N'} = $type_bit{'u'};
-    $type_bit{'U'} = $type_bit{'u'};
-    $type_bit{'e'} = $type_bit{'u'};
-    $type_bit{'P'} = $type_bit{'u'};
-    $type_bit{'Q'} = $type_bit{'u'};
+    define_type_bit('I', $type_bit{'u'});
+    define_type_bit('t', $type_bit{'u'});
+    define_type_bit('A', $type_bit{'u'});
+    define_type_bit('L', $type_bit{'u'});
+    define_type_bit('b', $type_bit{'u'});
+    define_type_bit('N', $type_bit{'u'});
+    define_type_bit('U', $type_bit{'u'});
+    define_type_bit('e', $type_bit{'u'});
+    define_type_bit('P', $type_bit{'u'});
+    define_type_bit('Q', $type_bit{'u'});
+}
+
+#
+# Sanity checks.
+#
+
+{
+    if (@tag_type > $max_genop_types) {
+	sanity("\$max_genop_types is $max_genop_types, ",
+	       "but there are ", scalar(@tag_type),
+	       " primitive tags defined\n");
+    }
+
+    foreach my $tag (@tag_type) {
+	sanity("tag '$tag': primitive tags must be named with lowercase letters")
+	    unless $tag =~ /^[a-z]$/;
+    }
 }
 
 #
@@ -436,12 +467,12 @@ sub emulator_output {
 	    #
 
 	    my(@bits) = (0) x ($max_spec_operands/2);
-	    my($shift) = 16;
 	    my($i);
 	    for ($i = 0; $i < $max_spec_operands && defined $args[$i]; $i++) {
 		my $t = $args[$i];
 		if (defined $type_bit{$t}) {
-		    $bits[int($i/2)] |= $type_bit{$t} << (16*($i%2));
+		    my $shift = $max_genop_types * ($i % 2);
+		    $bits[int($i/2)] |= $type_bit{$t} << $shift;
 		}
 	    }
 
@@ -753,6 +784,10 @@ sub error {
     die $where, @message, "\n";
 }
 
+sub sanity {
+    die "internal error: ", @_, "\n";
+}
+
 sub comment {
     my($lang, @comments) = @_;
     my($prefix);