From e5f3fea6dbcdf41d5c6d4c00a7d1c85817cf4c01 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, that 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 | 21 +++++-- 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, 110 insertions(+), 9 deletions(-) diff --git a/lib/compiler/src/beam_ssa.erl b/lib/compiler/src/beam_ssa.erl index 503781161420..c1d11c89da06 100644 --- a/lib/compiler/src/beam_ssa.erl +++ b/lib/compiler/src/beam_ssa.erl @@ -372,13 +372,22 @@ 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 - {false,_} -> - Set; - {true,[#b_literal{}=Lit,#b_var{}=Var]} -> - Set#b_set{args=[Var,Lit]}; - {true,_} -> + {true, [#b_literal{}=Lit,#b_var{}=Var]} -> + 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; normalize(#b_set{}=Set) -> diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl index 98eacce0fc14..95243feeb199 100644 --- a/lib/compiler/src/beam_validator.erl +++ b/lib/compiler/src/beam_validator.erl @@ -1463,7 +1463,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) -> 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 d1330b78bf9d..b98ff3644c05 100644 --- a/lib/compiler/test/beam_ssa_SUITE.erl +++ b/lib/compiler/test/beam_ssa_SUITE.erl @@ -26,7 +26,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]}]. @@ -47,7 +47,8 @@ groups() -> stack_init, grab_bag, redundant_br, - coverage + coverage, + normalize ]}]. init_per_suite(Config) -> @@ -1178,5 +1179,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 ceb7ff07b3ab..e448b5cf41d8 100644 --- a/lib/compiler/test/beam_type_SUITE.erl +++ b/lib/compiler/test/beam_type_SUITE.erl @@ -499,11 +499,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),