Merge pull request #11480 from bjorng/bjorn/compiler/fix-rem-bounds/GH-11400

Fix bound calculation for the rem operator

Author: bjorng

lib/compiler/src/beam_bounds.erl

@@ -230,28 +230,29 @@ div_bounds({A,B}, _) when is_integer(A), is_integer(B) ->
 div_bounds(_, _) ->
     any.
 
-rem_bounds({A,_}, {C,D}) when is_integer(C), is_integer(D), C > 0 ->
-    Max = inf_add(D, -1),
-    Min = if
-              A =:= '-inf' -> -Max;
-              A >= 0 -> 0;
-              true -> -Max
-          end,
-    normalize({Min,Max});
-rem_bounds(_, {C,D}) when is_integer(C), is_integer(D),
-                     C =/= 0 orelse D =/= 0 ->
-    Max = max(abs(C), abs(D)) - 1,
-    Min = -Max,
-    normalize({Min,Max});
-rem_bounds({A,B}, _) ->
+rem_bounds({A,B}, {C,D}) ->
+    MaxC = inf_max(inf_add(inf_abs(C), -1), 0),
+    MaxD = inf_max(inf_add(inf_abs(D), -1), 0),
+    MaxCD = inf_max(MaxC, MaxD),
+    Min = inf_max(A, inf_neg(MaxCD)),
+    Max = inf_min(B, MaxCD),
+
     %% The sign of the remainder is the same as the sign of the
     %% left-hand side operand; it does not depend on the sign of the
-    %% right-hand side operand. Therefore, the range of the remainder
-    %% is the range of the left-hand side operand extended to always
-    %% include zero.
-    Min = inf_min(0, A),
-    Max = inf_max(0, B),
-    normalize({Min,Max}).
+    %% right-hand side operand.
+    case {inf_sign(A),inf_sign(B)} of
+        {'-','-'} ->
+            %% The LHS operand is always negative; cap maximum at
+            %% zero.
+            normalize({Min,0});
+        {'-','+'} ->
+            %% The LHS operand can be negative or positive.
+            normalize({Min,Max});
+        {'+','+'} ->
+            %% The LHS operand is always non-negative; cap minimum at
+            %% zero.
+            normalize({0,Max})
+    end.
 
 min_max_band(A, B, C, D) ->
     {Min,Max} = min_max_bor(inf_bnot(B), inf_bnot(A),

lib/compiler/test/beam_bounds_SUITE.erl

@@ -200,11 +200,14 @@ rem_bounds(_Config) ->
 
     {-7,7} = beam_bounds:bounds('rem', {'-inf',10}, {1,8}),
     {0,7} = beam_bounds:bounds('rem', {10,'+inf'}, {1,8}),
+    {0,9} = beam_bounds:bounds('rem', {10,'+inf'}, {-10,7}),
     {0,'+inf'} = beam_bounds:bounds('rem', {17,'+inf'}, any),
+    {-5,9} = beam_bounds:bounds('rem', {-5,'+inf'}, {-10,7}),
 
     {0,10} = beam_bounds:bounds('rem', {1,10}, {'-inf',10}),
     {0,'+inf'} = beam_bounds:bounds('rem', {20,'+inf'}, {10,'+inf'}),
     {'-inf',10} = beam_bounds:bounds('rem', {'-inf',10}, any),
+    {-16,10} = beam_bounds:bounds('rem', {'-inf',10}, {-17,5}),
 
     {-11,10} = beam_bounds:bounds('rem', {-11,10}, {'-inf',89}),
     {-11,10} = beam_bounds:bounds('rem', {-11,10}, {7,'+inf'}),
@@ -226,6 +229,13 @@ rem_bounds(_Config) ->
     {'-inf',0} = beam_bounds:bounds('rem', {'-inf',-5}, any),
     {'-inf',11} = beam_bounds:bounds('rem', {'-inf',11}, any),
 
+    {0,1} = beam_bounds:bounds('rem', {1,1}, any),
+    {0,1} = beam_bounds:bounds('rem', {1,1}, {-10,7}),
+    {-1,1} = beam_bounds:bounds('rem', {-1,1}, any),
+    {-1,1} = beam_bounds:bounds('rem', {-1,1}, {-7,10}),
+    {-2,0} = beam_bounds:bounds('rem', {-2,-1}, any),
+    {-2,0} = beam_bounds:bounds('rem', {-2,-1}, {-7,10}),
+
     any = beam_bounds:bounds('rem', any, {'-inf',-7}),
     any = beam_bounds:bounds('rem', any, {'-inf',0}),
     any = beam_bounds:bounds('rem', any, {'-inf',1}),

lib/compiler/test/beam_type_SUITE.erl

@@ -36,7 +36,8 @@
          cover_maps_functions/1,min_max_mixed_types/1,
          not_equal/1,infer_relops/1,binary_unit/1,premature_concretization/1,
          funs/1,will_succeed/1,float_confusion/1,
-         cover_convert_ext/1, catch_setelement/1,gh_11368/1]).
+         cover_convert_ext/1, catch_setelement/1,gh_11368/1,
+         rem_bounds/1]).
 
 %% Force id/1 to return 'any'.
 -export([id/1]).
@@ -86,7 +87,8 @@ groups() ->
        float_confusion,
        cover_convert_ext,
        catch_setelement,
-       gh_11368
+       gh_11368,
+       rem_bounds
       ]}].
 
 init_per_suite(Config) ->
@@ -1677,6 +1679,20 @@ prepare_request(Msg0, Seq) ->
     Msg1 = setelement(3, Msg0, [request | Flags]),
     setelement(4, Msg1, Seq).
 
+rem_bounds(_Config) ->
+    %% Don't call the rem_bounds_1/1 function, because it will never
+    %% terminate.
+    case id(ignore) of
+        call -> rem_bounds_1(1);
+        ignore -> ok
+    end,
+
+    ok.
+
+%% GH-11400. beam_validator would reject the following code.
+rem_bounds_1(N) ->
+    rem_bounds_1((1 rem N) - 2).
+
 %%%
 %%% Common utilities.
 %%%