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 `<<F/float>>` succeeded, then `F` must be a float. That is not correct, because `F` could also be an integer. Closes #7147
erlang · May 2, 2023 · e5f3fea · e5f3fea
1 parent 5400ccf
commit e5f3fea
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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>>) ->
     <<-Float/float>>.
 
+%% GH-7147.
+binary_float_1(X, Y) ->
+    _ = <<Y:(ceil(64.0 = X))/float, (binary_to_integer(ok))>>,
+    ceil(X) band Y.
+
 float_compare(_Config) ->
     false = do_float_compare(-42.0),
     false = do_float_compare(-42),