From b178b93c7b731d127486fd82e0c5118d3795b88c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Mon, 24 Apr 2023 11:49:20 +0200 Subject: [PATCH] Fix two type-related bugs Issue #7147 uncovered two bugs in the compiler. The first one is in the swapping of operands for commutative operands, which the compiler does to simplify for the JIT. When the operands were swapped, types for the operands in the annotation were not updated. The second one is in `beam_validator`, which assumed that if the binary construction `<>` succeeded, then `F` must be a float. That is not correct, because `F` could also be an integer. Closes #7147 --- lib/compiler/src/beam_ssa.erl | 15 ++++- lib/compiler/src/beam_validator.erl | 2 +- lib/compiler/test/beam_ssa_SUITE.erl | 90 ++++++++++++++++++++++++++- lib/compiler/test/beam_type_SUITE.erl | 6 ++ 4 files changed, 108 insertions(+), 5 deletions(-) diff --git a/lib/compiler/src/beam_ssa.erl b/lib/compiler/src/beam_ssa.erl index 619a9f032a84..448b3b431315 100644 --- a/lib/compiler/src/beam_ssa.erl +++ b/lib/compiler/src/beam_ssa.erl @@ -383,10 +383,21 @@ successors(#b_blk{last=Terminator}) -> -spec normalize(b_set() | terminator()) -> b_set() | terminator(). -normalize(#b_set{op={bif,Bif},args=Args}=Set) -> +normalize(#b_set{anno=Anno0,op={bif,Bif},args=Args}=Set) -> case {is_commutative(Bif),Args} of {true, [#b_literal{}=Lit,#b_var{}=Var]} -> - Set#b_set{args=[Var,Lit]}; + Anno = case Anno0 of + #{arg_types := ArgTypes0} -> + case ArgTypes0 of + #{1 := Type} -> + Anno0#{arg_types => #{0 => Type}}; + #{} -> + Anno0 + end; + #{} -> + Anno0 + end, + Set#b_set{anno=Anno,args=[Var,Lit]}; {_, _} -> Set end; diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl index f19ea897fe15..ee176b085f06 100644 --- a/lib/compiler/src/beam_validator.erl +++ b/lib/compiler/src/beam_validator.erl @@ -1582,7 +1582,7 @@ update_create_bin_list([], Vst) -> Vst. update_create_bin_type(append) -> #t_bitstring{}; update_create_bin_type(private_append) -> #t_bitstring{}; update_create_bin_type(binary) -> #t_bitstring{}; -update_create_bin_type(float) -> #t_float{}; +update_create_bin_type(float) -> #t_number{}; update_create_bin_type(integer) -> #t_integer{}; update_create_bin_type(utf8) -> #t_integer{}; update_create_bin_type(utf16) -> #t_integer{}; diff --git a/lib/compiler/test/beam_ssa_SUITE.erl b/lib/compiler/test/beam_ssa_SUITE.erl index 6a91bdd7b82c..0bb485c7f1bb 100644 --- a/lib/compiler/test/beam_ssa_SUITE.erl +++ b/lib/compiler/test/beam_ssa_SUITE.erl @@ -27,7 +27,7 @@ beam_ssa_dead_crash/1,stack_init/1, mapfoldl/0,mapfoldl/1, grab_bag/1,redundant_br/1, - coverage/1]). + coverage/1,normalize/1]). suite() -> [{ct_hooks,[ts_install_cth]}]. @@ -48,7 +48,8 @@ groups() -> stack_init, grab_bag, redundant_br, - coverage + coverage, + normalize ]}]. init_per_suite(Config) -> @@ -1260,5 +1261,90 @@ coverage_5() -> error end#coverage{name = whatever}. +%% Test beam_ssa:normalize/1, especially that argument types are +%% correctly updated when arguments are swapped. +normalize(_Config) -> + normalize_commutative({bif,'band'}), + normalize_commutative({bif,'+'}), + + normalize_noncommutative({bif,'div'}), + + ok. + +-record(b_var, {name}). +-record(b_literal, {val}). + +normalize_commutative(Op) -> + A = #b_var{name=a}, + B = #b_var{name=b}, + Lit = #b_literal{val=42}, + + normalize_same(Op, [A,B]), + normalize_same(Op, [A,Lit]), + + normalize_swapped(Op, [Lit,A]), + + ok. + +normalize_noncommutative(Op) -> + A = #b_var{name=a}, + B = #b_var{name=b}, + Lit = #b_literal{val=42}, + + normalize_same(Op, [A,B]), + normalize_same(Op, [A,Lit]), + + ArgTypes0 = [{1,beam_types:make_integer(0, 1023)}], + I1 = make_bset(ArgTypes0, Op, [Lit,A]), + I1 = beam_ssa:normalize(I1), + + ok. + +normalize_same(Op, Args) -> + I0 = make_bset(#{}, Op, Args), + I0 = beam_ssa:normalize(I0), + + ArgTypes0 = [{0,beam_types:make_integer(0, 1023)}], + I1 = make_bset(ArgTypes0, Op, Args), + I1 = beam_ssa:normalize(I1), + + case Args of + [#b_var{},#b_var{}] -> + ArgTypes1 = [{0,beam_types:make_integer(0, 1023)}, + {1,beam_types:make_integer(42)}], + I2 = make_bset(ArgTypes1, Op, Args), + I2 = beam_ssa:normalize(I2); + [_,_] -> + ok + end, + + ok. + +normalize_swapped(Op, [#b_literal{}=Lit,#b_var{}=Var]=Args) -> + EmptyAnno = #{}, + I0 = make_bset(EmptyAnno, Op, Args), + {b_set,EmptyAnno,#b_var{name=1000},Op,[Var,Lit]} = beam_ssa:normalize(I0), + + EmptyTypes = #{arg_types => #{}}, + I1 = make_bset(EmptyTypes, Op, Args), + {b_set,EmptyTypes,#b_var{name=1000},Op,[Var,Lit]} = beam_ssa:normalize(I1), + + IntRange = beam_types:make_integer(0, 1023), + ArgTypes0 = [{1,IntRange}], + I2 = make_bset(ArgTypes0, Op, Args), + {[{0,IntRange}],Op,[Var,Lit]} = unpack_bset(beam_ssa:normalize(I2)), + + ok. + +make_bset(ArgTypes, Op, Args) when is_list(ArgTypes) -> + Anno = #{arg_types => maps:from_list(ArgTypes)}, + {b_set,Anno,#b_var{name=1000},Op,Args}; +make_bset(Anno, Op, Args) when is_map(Anno) -> + {b_set,Anno,#b_var{name=1000},Op,Args}. + +unpack_bset({b_set,Anno,{b_var,1000},Op,Args}) -> + ArgTypes = maps:get(arg_types, Anno, #{}), + {lists:sort(maps:to_list(ArgTypes)),Op,Args}. + %% The identity function. id(I) -> I. diff --git a/lib/compiler/test/beam_type_SUITE.erl b/lib/compiler/test/beam_type_SUITE.erl index 0b00e6d71ca5..6562b3228f3f 100644 --- a/lib/compiler/test/beam_type_SUITE.erl +++ b/lib/compiler/test/beam_type_SUITE.erl @@ -741,11 +741,17 @@ record_float(R, N0) -> binary_float(_Config) -> <<-1/float>> = binary_negate_float(<<1/float>>), + {'EXIT',{badarg,_}} = catch binary_float_1(id(64.0), id(0)), ok. binary_negate_float(<>) -> <<-Float/float>>. +%% GH-7147. +binary_float_1(X, Y) -> + _ = <>, + ceil(X) band Y. + float_compare(_Config) -> false = do_float_compare(-42.0), false = do_float_compare(-42),