Skip to content

Commit

Permalink
Merge branch 'margnus1/bs_unit_fix' into maint
Browse files Browse the repository at this point in the history
* margnus1/bs_unit_fix:
  hipe: Fix signed compares of unsigned sizes
  beam: Fix overflow bug in i_bs_add_jId
  hipe: Add tests for bad bit syntax float sizes
  Add a case testing the handling of guards involving binaries
  Add some more binary syntax construction tests
  hipe: Guard against enormous numbers in ranges
  hipe: Fix constructing huge binaries
  hipe: Fix binary constructions failing with badarith
  Add missing corner-case to bs_construct_SUITE
  hipe: Allow unsigned args in hipe_rtl_arith
  hipe: test unit size match in bs_put_binary_all
  hipe: test unit size match in bs_append
  Fix hipe_rtl_binary_construct:floorlog2/1

OTP-13272
  • Loading branch information
zhird committed Feb 2, 2016
2 parents 7cb403e + 34380ba commit 61ef751
Show file tree
Hide file tree
Showing 19 changed files with 521 additions and 315 deletions.
2 changes: 1 addition & 1 deletion erts/emulator/beam/beam_emu.c
Expand Up @@ -4069,7 +4069,7 @@ do { \
tmp_arg1 += Arg1;

store_bs_add_result:
if (MY_IS_SSMALL((Sint) tmp_arg1)) {
if (tmp_arg1 <= MAX_SMALL) {
tmp_arg1 = make_small(tmp_arg1);
} else {
/*
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/hipe/hipe_bif0.tab
Expand Up @@ -142,4 +142,4 @@ atom bs_validate_unicode
atom bs_validate_unicode_retract
atom emulate_fpe
atom emasculate_binary

atom is_divisible
1 change: 1 addition & 0 deletions erts/emulator/hipe/hipe_bif_list.m4
Expand Up @@ -193,6 +193,7 @@ standard_bif_interface_2(nbif_rethrow, hipe_rethrow)
standard_bif_interface_3(nbif_find_na_or_make_stub, hipe_find_na_or_make_stub)
standard_bif_interface_2(nbif_nonclosure_address, hipe_nonclosure_address)
nocons_nofail_primop_interface_0(nbif_fclearerror_error, hipe_fclearerror_error)
standard_bif_interface_2(nbif_is_divisible, hipe_is_divisible)

/*
* Mbox primops with implicit P parameter.
Expand Down
12 changes: 12 additions & 0 deletions erts/emulator/hipe/hipe_native_bif.c
Expand Up @@ -504,6 +504,18 @@ int hipe_bs_validate_unicode_retract(ErlBinMatchBuffer* mb, Eterm arg)
return 1;
}

BIF_RETTYPE hipe_is_divisible(BIF_ALIST_2)
{
/* Arguments are Eterm-sized unsigned integers */
Uint dividend = BIF_ARG_1;
Uint divisor = BIF_ARG_2;
if (dividend % divisor) {
BIF_ERROR(BIF_P, BADARG);
} else {
return NIL;
}
}

/* This is like the loop_rec_fr BEAM instruction
*/
Eterm hipe_check_get_msg(Process *c_p)
Expand Down
2 changes: 2 additions & 0 deletions erts/emulator/hipe/hipe_native_bif.h
Expand Up @@ -68,6 +68,7 @@ AEXTERN(Eterm,nbif_bs_put_utf16le,(Process*,Eterm,byte*,unsigned int));
AEXTERN(Eterm,nbif_bs_get_utf16,(void));
AEXTERN(Eterm,nbif_bs_validate_unicode,(Process*,Eterm));
AEXTERN(Eterm,nbif_bs_validate_unicode_retract,(void));
AEXTERN(void,nbif_is_divisible,(Process*,Uint,Uint));

AEXTERN(void,nbif_select_msg,(Process*));
AEXTERN(Eterm,nbif_cmp_2,(void));
Expand All @@ -93,6 +94,7 @@ BIF_RETTYPE hipe_bs_put_utf16le(BIF_ALIST_3);
BIF_RETTYPE hipe_bs_validate_unicode(BIF_ALIST_1);
struct erl_bin_match_buffer;
int hipe_bs_validate_unicode_retract(struct erl_bin_match_buffer*, Eterm);
BIF_RETTYPE hipe_is_divisible(BIF_ALIST_2);

#ifdef NO_FPE_SIGNALS
AEXTERN(void,nbif_emulate_fpe,(Process*));
Expand Down
2 changes: 2 additions & 0 deletions erts/emulator/hipe/hipe_primops.h
Expand Up @@ -68,6 +68,8 @@ PRIMOP_LIST(am_bs_get_utf16, &nbif_bs_get_utf16)
PRIMOP_LIST(am_bs_validate_unicode, &nbif_bs_validate_unicode)
PRIMOP_LIST(am_bs_validate_unicode_retract, &nbif_bs_validate_unicode_retract)

PRIMOP_LIST(am_is_divisible, &nbif_is_divisible)

PRIMOP_LIST(am_cmp_2, &nbif_cmp_2)
PRIMOP_LIST(am_op_exact_eqeq_2, &nbif_eq_2)

Expand Down
40 changes: 34 additions & 6 deletions erts/emulator/test/bs_construct_SUITE.erl
Expand Up @@ -29,7 +29,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, bad_append/1]).
otp_7422/1, zero_width/1, bad_append/1, bs_add_overflow/1]).

-include_lib("test_server/include/test_server.hrl").

Expand All @@ -40,7 +40,7 @@ all() ->
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,
bad_append].
bad_append, bs_add_overflow].

groups() ->
[].
Expand Down Expand Up @@ -551,10 +551,24 @@ huge_binary(Config) when is_list(Config) ->
?line 16777216 = size(<<0:(id(1 bsl 26)),(-1):(id(1 bsl 26))>>),
?line garbage_collect(),
{Shift,Return} = case free_mem() of
undefined -> {32,ok};
Mb when Mb > 600 -> {32,ok};
Mb when Mb > 300 -> {31,"Limit huge binaries to 256 Mb"};
_ -> {30,"Limit huge binary to 128 Mb"}
undefined ->
%% This test has to be inlined inside the case to
%% use a literal Shift
?line garbage_collect(),
?line id(<<0:((1 bsl 32)-1)>>),
{32,ok};
Mb when Mb > 600 ->
?line garbage_collect(),
?line id(<<0:((1 bsl 32)-1)>>),
{32,ok};
Mb when Mb > 300 ->
?line garbage_collect(),
?line id(<<0:((1 bsl 31)-1)>>),
{31,"Limit huge binaries to 256 Mb"};
_ ->
?line garbage_collect(),
?line id(<<0:((1 bsl 30)-1)>>),
{30,"Limit huge binary to 128 Mb"}
end,
?line garbage_collect(),
?line id(<<0:((1 bsl Shift)-1)>>),
Expand Down Expand Up @@ -911,5 +925,19 @@ append_unit_8(Bin) ->
append_unit_16(Bin) ->
<<Bin/binary-unit:16,0:1>>.

%% 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 ->
{skip, "64-bit architecture"};
4 ->
Large = <<0:((1 bsl 30)-1)>>,
{'EXIT',{system_limit,_}} =
(catch <<Large/bits, Large/bits, Large/bits, Large/bits,
Large/bits, Large/bits, Large/bits, Large/bits,
Large/bits>>),
ok
end.

id(I) -> I.
8 changes: 4 additions & 4 deletions lib/hipe/cerl/cerl_to_icode.erl
Expand Up @@ -794,9 +794,9 @@ bitstr_gen_op([V], #ctxt{fail=FL, class=guard}, SizeInfo, ConstInfo,
Type, Flags, Base, Offset) ->
SL = new_label(),
case SizeInfo of
{all,_NewUnit, NewAlign, S1} ->
{all, NewUnit, NewAlign, S1} ->
Type = binary,
Name = {bs_put_binary_all, Flags},
Name = {bs_put_binary_all, NewUnit, Flags},
Primop = {hipe_bs_primop, Name},
{add_code([icode_guardop([Offset], Primop,
[V, Base, Offset], SL, FL),
Expand All @@ -819,9 +819,9 @@ bitstr_gen_op([V], #ctxt{fail=FL, class=guard}, SizeInfo, ConstInfo,
bitstr_gen_op([V], _Ctxt, SizeInfo, ConstInfo, Type, Flags, Base,
Offset) ->
case SizeInfo of
{all, _NewUnit, NewAlign, S} ->
{all, NewUnit, NewAlign, S} ->
Type = binary,
Name = {bs_put_binary_all, Flags},
Name = {bs_put_binary_all, NewUnit, Flags},
Primop = {hipe_bs_primop, Name},
{add_code([icode_call_primop([Offset], Primop,
[V, Base, Offset])], S),
Expand Down
51 changes: 23 additions & 28 deletions lib/hipe/icode/hipe_beam_to_icode.erl
Expand Up @@ -881,6 +881,15 @@ trans_fun([{bs_init_bits,{f,Lbl},Size,_Words,_LiveRegs,{field_flags,Flags0},X}|
trans_fun([{bs_add, {f,Lbl}, [Old,New,Unit], Res}|Instructions], Env) ->
Dst = mk_var(Res),
Temp = mk_var(new),
{FailLblName, FailCode} =
if Lbl =:= 0 ->
FailLbl = mk_label(new),
{hipe_icode:label_name(FailLbl),
[FailLbl,
hipe_icode:mk_fail([hipe_icode:mk_const(badarg)], error)]};
true ->
{map_label(Lbl), []}
end,
MultIs =
case {New,Unit} of
{{integer, NewInt}, _} ->
Expand All @@ -890,40 +899,26 @@ trans_fun([{bs_add, {f,Lbl}, [Old,New,Unit], Res}|Instructions], Env) ->
[hipe_icode:mk_move(Temp, NewVar)];
_ ->
NewVar = mk_var(New),
if Lbl =:= 0 ->
[hipe_icode:mk_primop([Temp], '*',
[NewVar, hipe_icode:mk_const(Unit)])];
true ->
Succ = mk_label(new),
[hipe_icode:mk_primop([Temp], '*',
[NewVar, hipe_icode:mk_const(Unit)],
hipe_icode:label_name(Succ), map_label(Lbl)),
Succ]
end
Succ = mk_label(new),
[hipe_icode:mk_primop([Temp], '*',
[NewVar, hipe_icode:mk_const(Unit)],
hipe_icode:label_name(Succ), FailLblName),
Succ]
end,
Succ2 = mk_label(new),
{FailLblName, FailCode} =
if Lbl =:= 0 ->
FailLbl = mk_label(new),
{hipe_icode:label_name(FailLbl),
[FailLbl,
hipe_icode:mk_fail([hipe_icode:mk_const(badarg)], error)]};
true ->
{map_label(Lbl), []}
end,
IsPos =
[hipe_icode:mk_if('>=', [Temp, hipe_icode:mk_const(0)],
hipe_icode:label_name(Succ2), FailLblName)] ++
FailCode ++ [Succ2],
AddI =
FailCode ++ [Succ2],
AddRhs =
case Old of
{integer,OldInt} ->
hipe_icode:mk_primop([Dst], '+', [Temp, hipe_icode:mk_const(OldInt)]);
_ ->
OldVar = mk_var(Old),
hipe_icode:mk_primop([Dst], '+', [Temp, OldVar])
{integer,OldInt} -> hipe_icode:mk_const(OldInt);
_ -> mk_var(Old)
end,
MultIs ++ IsPos ++ [AddI|trans_fun(Instructions, Env)];
Succ3 = mk_label(new),
AddI = hipe_icode:mk_primop([Dst], '+', [Temp, AddRhs],
hipe_icode:label_name(Succ3), FailLblName),
MultIs ++ IsPos ++ [AddI,Succ3|trans_fun(Instructions, Env)];
%%--------------------------------------------------------------------
%% Bit syntax instructions added in R12B-5 (Fall 2008)
%%--------------------------------------------------------------------
Expand Down Expand Up @@ -1306,7 +1301,7 @@ trans_bin([{bs_put_binary,{f,Lbl},Size,Unit,{field_flags,Flags},Source}|
{Name, Args, Env2} =
case Size of
{atom,all} -> %% put all bits
{{bs_put_binary_all, Flags}, [Src,Base,Offset], Env};
{{bs_put_binary_all, Unit, Flags}, [Src,Base,Offset], Env};
{integer,NoBits} when is_integer(NoBits), NoBits >= 0 ->
%% Create a N*Unit bits subbinary
{{bs_put_binary, NoBits*Unit, Flags}, [Src,Base,Offset], Env};
Expand Down
37 changes: 28 additions & 9 deletions lib/hipe/icode/hipe_icode_primops.erl
Expand Up @@ -116,7 +116,7 @@ is_safe({hipe_bs_primop, {bs_init, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_init_bits, _}}) -> false;
is_safe({hipe_bs_primop, {bs_init_bits, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_binary, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_binary_all, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_binary_all, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_float, _, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_integer, _, _, _}}) -> false;
is_safe({hipe_bs_primop, {bs_put_string, _, _}}) -> false;
Expand Down Expand Up @@ -219,7 +219,7 @@ fails({hipe_bs_primop, {bs_init, _, _}}) -> true;
fails({hipe_bs_primop, {bs_init_bits, _}}) -> true;
fails({hipe_bs_primop, {bs_init_bits, _, _}}) -> true;
fails({hipe_bs_primop, {bs_put_binary, _, _}}) -> true;
fails({hipe_bs_primop, {bs_put_binary_all, _}}) -> true;
fails({hipe_bs_primop, {bs_put_binary_all, _, _}}) -> true;
fails({hipe_bs_primop, {bs_put_float, _, _, _}}) -> true;
fails({hipe_bs_primop, {bs_put_integer, _, _, _}}) -> true;
fails({hipe_bs_primop, {bs_put_string, _, _}}) -> true;
Expand Down Expand Up @@ -265,8 +265,8 @@ pp(Dev, Op) ->
io:format(Dev, "gc_test<~w>", [N]);
{hipe_bs_primop, BsOp} ->
case BsOp of
{bs_put_binary_all, Flags} ->
io:format(Dev, "bs_put_binary_all<~w>", [Flags]);
{bs_put_binary_all, Unit, Flags} ->
io:format(Dev, "bs_put_binary_all<~w, ~w>", [Unit,Flags]);
{bs_put_binary, Size} ->
io:format(Dev, "bs_put_binary<~w>", [Size]);
{bs_put_binary, Flags, Size} ->
Expand Down Expand Up @@ -505,11 +505,13 @@ type(Primop, Args) ->
NewMatchState =
erl_types:t_matchstate_update_present(NewBinType, MatchState),
if Signed =:= 0 ->
erl_types:t_product([erl_types:t_from_range(0, 1 bsl Size - 1),
UpperBound = inf_add(safe_bsl(1, Size), -1),
erl_types:t_product([erl_types:t_from_range(0, UpperBound),
NewMatchState]);
Signed =:= 4 ->
erl_types:t_product([erl_types:t_from_range(- (1 bsl (Size-1)),
(1 bsl (Size-1)) - 1),
erl_types:t_product([erl_types:t_from_range(
inf_inv(safe_bsl(1, Size-1)),
inf_add(safe_bsl(1, Size-1), -1)),
NewMatchState])
end;
[_Arg] ->
Expand Down Expand Up @@ -628,8 +630,9 @@ type(Primop, Args) ->
[_SrcType, _BitsType, _Base, Type] ->
erl_types:t_bitstr_concat(Type, erl_types:t_bitstr(Size, 0))
end;
{hipe_bs_primop, {bs_put_binary_all, _Flags}} ->
[SrcType, _Base, Type] = Args,
{hipe_bs_primop, {bs_put_binary_all, Unit, _Flags}} ->
[SrcType0, _Base, Type] = Args,
SrcType = erl_types:t_inf(erl_types:t_bitstr(Unit, 0), SrcType0),
erl_types:t_bitstr_concat(SrcType,Type);
{hipe_bs_primop, {bs_put_string, _, Size}} ->
[_Base, Type] = Args,
Expand Down Expand Up @@ -965,3 +968,19 @@ check_fun_args(_, _) ->

match_bin(Pattern, Match) ->
erl_types:t_bitstr_match(Pattern, Match).

safe_bsl(0, _) -> 0;
safe_bsl(Base, Shift) when Shift =< 128 -> Base bsl Shift;
safe_bsl(Base, _Shift) when Base > 0 -> pos_inf;
safe_bsl(Base, _Shift) when Base < 0 -> neg_inf.

inf_inv(pos_inf) -> neg_inf;
inf_inv(neg_inf) -> pos_inf;
inf_inv(Number) -> -Number.

inf_add(pos_inf, _Number) -> pos_inf;
inf_add(neg_inf, _Number) -> neg_inf;
inf_add(_Number, pos_inf) -> pos_inf;
inf_add(_Number, neg_inf) -> neg_inf;
inf_add(Number1, Number2) when is_integer(Number1), is_integer(Number2) ->
Number1 + Number2.
6 changes: 3 additions & 3 deletions lib/hipe/icode/hipe_icode_range.erl
Expand Up @@ -1202,11 +1202,11 @@ basic_type(#unsafe_update_element{}) -> not_analysed.
analyse_bs_get_integer(Size, Flags, true) ->
Signed = Flags band 4,
if Signed =:= 0 ->
Max = 1 bsl Size - 1,
Max = inf_add(inf_bsl(1, Size), -1),
Min = 0;
true ->
Max = 1 bsl (Size-1) - 1,
Min = -(1 bsl (Size-1))
Max = inf_add(inf_bsl(1, Size-1), -1),
Min = inf_inv(inf_bsl(1, Size-1))
end,
{Min, Max};
analyse_bs_get_integer(Size, Flags, false) when is_integer(Size),
Expand Down
15 changes: 4 additions & 11 deletions lib/hipe/rtl/hipe_rtl_arith.inc
Expand Up @@ -30,13 +30,13 @@
%% Returns a tuple
%% {Res, Sign, Zero, Overflow, Carry}
%% Res will be a number in the range
%% MAX_SIGNED_INT >= Res >= MIN_SIGNED_INT
%% MAX_UNSIGNED_INT >= Res >= 0
%% The other four values are flags that are either true or false
%%
eval_alu(Op, Arg1, Arg2)
when Arg1 =< ?MAX_SIGNED_INT,
when Arg1 =< ?MAX_UNSIGNED_INT,
Arg1 >= ?MIN_SIGNED_INT,
Arg2 =< ?MAX_SIGNED_INT,
Arg2 =< ?MAX_UNSIGNED_INT,
Arg2 >= ?MIN_SIGNED_INT ->

Sign1 = sign_bit(Arg1),
Expand Down Expand Up @@ -111,7 +111,7 @@ eval_alu(Op, Arg1, Arg2)
Res = N = Z = V = C = 0,
?EXIT({"unknown alu op", Op})
end,
{two_comp_to_erl(Res), N, Z, V, C};
{Res, N, Z, V, C};
eval_alu(Op, Arg1, Arg2) ->
?EXIT({argument_overflow,Op,Arg1,Arg2}).

Expand Down Expand Up @@ -162,16 +162,9 @@ eval_cond(Cond, Arg1, Arg2) ->
sign_bit(Val) ->
((Val bsr ?SIGN_BIT) band 1) =:= 1.

two_comp_to_erl(V) ->
if V > ?MAX_SIGNED_INT ->
- ((?MAX_UNSIGNED_INT + 1) - V);
true -> V
end.

shiftmask(Arg) ->
Setbits = ?BITS - Arg,
(1 bsl Setbits) - 1.

zero(Val) ->
Val =:= 0.

3 changes: 2 additions & 1 deletion lib/hipe/rtl/hipe_rtl_arith_32.erl
Expand Up @@ -24,7 +24,8 @@
%% Filename : hipe_rtl_arith_32.erl
%% Module : hipe_rtl_arith_32
%% Purpose : To implement 32-bit RTL-arithmetic
%% Notes : The arithmetic works on 32-bit signed integers.
%% Notes : The arithmetic works on 32-bit signed and unsigned
%% integers.
%% The implementation is taken from the implementation
%% of arithmetic on SPARC.
%% XXX: This code is seldom used, and hence also
Expand Down

0 comments on commit 61ef751

Please sign in to comment.