From 32ae1b4404b200d4a033d34920e09854770be042 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Fri, 8 Mar 2019 11:27:49 +0100
Subject: beam_ssa_opt: Fix crash in ssa_opt_float

For reasons better explained in the source code, ssa_opt_float
skips optimizing inside guards but it failed to do so
consistently; while the pass never processed guard blocks, it was
still possible to erroneously defer error checking to a guard
block, crashing the compiler once it realized its state was
invalid.
---
 lib/compiler/src/beam_ssa_opt.erl | 32 +++++++++++++++++++-------------
 lib/compiler/test/float_SUITE.erl | 23 ++++++++++++++++++++---
 2 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl
index bcf55f3fda..90c0d3cf16 100644
--- a/lib/compiler/src/beam_ssa_opt.erl
+++ b/lib/compiler/src/beam_ssa_opt.erl
@@ -911,6 +911,11 @@ ssa_opt_float({#st{ssa=Linear0,cnt=Count0}=St, FuncDb}) ->
     {Linear,Count} = float_opt(Linear0, Count0, Fs),
     {St#st{ssa=Linear,cnt=Count}, FuncDb}.
 
+float_blk_is_in_guard(#b_blk{last=#b_br{fail=F}}, #fs{non_guards=NonGuards}) ->
+    not gb_sets:is_member(F, NonGuards);
+float_blk_is_in_guard(#b_blk{}, #fs{}) ->
+    false.
+
 float_non_guards([{L,#b_blk{is=Is}}|Bs]) ->
     case Is of
         [#b_set{op=landingpad}|_] ->
@@ -920,21 +925,18 @@ float_non_guards([{L,#b_blk{is=Is}}|Bs]) ->
     end;
 float_non_guards([]) -> [?BADARG_BLOCK].
 
-float_opt([{L,#b_blk{last=#b_br{fail=F}}=Blk}|Bs0],
-            Count0, #fs{non_guards=NonGuards}=Fs) ->
-    case gb_sets:is_member(F, NonGuards) of
+float_opt([{L,Blk}|Bs0], Count0, Fs) ->
+    case float_blk_is_in_guard(Blk, Fs) of
         true ->
-            %% This block is not inside a guard.
-            %% We can do the optimization.
-            float_opt_1(L, Blk, Bs0, Count0, Fs);
-        false ->
             %% This block is inside a guard. Don't do
             %% any floating point optimizations.
             {Bs,Count} = float_opt(Bs0, Count0, Fs),
-            {[{L,Blk}|Bs],Count}
+            {[{L,Blk}|Bs],Count};
+        false ->
+            %% This block is not inside a guard.
+            %% We can do the optimization.
+            float_opt_1(L, Blk, Bs0, Count0, Fs)
     end;
-float_opt([{L,Blk}|Bs], Count, Fs) ->
-    float_opt_1(L, Blk, Bs, Count, Fs);
 float_opt([], Count, _Fs) ->
     {[],Count}.
 
@@ -1010,10 +1012,14 @@ float_conv([{L,#b_blk{is=Is0}=Blk0}|Bs0], Fail, Count0) ->
 
 float_maybe_flush(Blk0, #fs{s=cleared,fail=Fail,bs=Blocks}=Fs0, Count0) ->
     #b_blk{last=#b_br{bool=#b_var{},succ=Succ}=Br} = Blk0,
-    #b_blk{is=Is} = map_get(Succ, Blocks),
+
+    %% If the success block starts with a floating point operation, we can
+    %% defer flushing to that block as long as it isn't a guard.
+    #b_blk{is=Is} = SuccBlk = map_get(Succ, Blocks),
+    SuccIsGuard = float_blk_is_in_guard(SuccBlk, Fs0),
+
     case Is of
-        [#b_set{anno=#{float_op:=_}}|_] ->
-            %% The next operation is also a floating point operation.
+        [#b_set{anno=#{float_op:=_}}|_] when not SuccIsGuard ->
             %% No flush needed.
             {[],Blk0,Fs0,Count0};
         _ ->
diff --git a/lib/compiler/test/float_SUITE.erl b/lib/compiler/test/float_SUITE.erl
index 831e8279aa..0fa8070dc8 100644
--- a/lib/compiler/test/float_SUITE.erl
+++ b/lib/compiler/test/float_SUITE.erl
@@ -21,15 +21,16 @@
 -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, 
 	 init_per_group/2,end_per_group/2,
 	 pending/1,bif_calls/1,math_functions/1,mixed_float_and_int/1,
-         subtract_number_type/1]).
+         subtract_number_type/1,float_followed_by_guard/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
-all() -> 
+all() ->
     [pending, bif_calls, math_functions,
-     mixed_float_and_int, subtract_number_type].
+     mixed_float_and_int, subtract_number_type,
+     float_followed_by_guard].
 
 groups() -> 
     [].
@@ -187,5 +188,21 @@ fact(0, P) -> P;
 fact(1, P) -> P;
 fact(N, P) -> fact(N-1, P*N).
 
+float_followed_by_guard(Config) when is_list(Config) ->
+    true = ffbg_1(5, 1),
+    false = ffbg_1(1, 5),
+    ok.
+
+ffbg_1(A, B0) ->
+    %% This is a non-guard block followed by a *guard block* that starts with a
+    %% floating point operation, and the compiler erroneously assumed that it
+    %% was safe to skip fcheckerror because the next block started with a float
+    %% op.
+    B = id(B0) / 1.0,
+    if
+        A - B > 0.0 -> true;
+        A - B =< 0.0 -> false
+    end.
+
 id(I) -> I.
 
-- 
cgit v1.2.3