aboutsummaryrefslogtreecommitdiffstats
path: root/lib
diff options
context:
space:
mode:
authorJohn Högberg <[email protected]>2019-03-08 11:27:49 +0100
committerJohn Högberg <[email protected]>2019-03-08 13:05:54 +0100
commit32ae1b4404b200d4a033d34920e09854770be042 (patch)
tree19cfb9a5fa8cffd91b6df6945d8ff60108f6cef5 /lib
parent31bff2cdb13a6e2032b7f067d037da0520752fc0 (diff)
downloadotp-32ae1b4404b200d4a033d34920e09854770be042.tar.gz
otp-32ae1b4404b200d4a033d34920e09854770be042.tar.bz2
otp-32ae1b4404b200d4a033d34920e09854770be042.zip
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.
Diffstat (limited to 'lib')
-rw-r--r--lib/compiler/src/beam_ssa_opt.erl32
-rw-r--r--lib/compiler/test/float_SUITE.erl23
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.