aboutsummaryrefslogtreecommitdiffstats
path: root/lib/hipe
diff options
context:
space:
mode:
authorMagnus Lång <[email protected]>2016-03-02 14:55:25 +0100
committerMagnus Lång <[email protected]>2016-03-02 15:52:31 +0100
commitc7cc7877e048eab4ee16169653d7cec51fe8a8df (patch)
treee6c7d752cd31354cc4951e47dc617d078db616f8 /lib/hipe
parentc3af7910226cabddcdfa1561b56127e71e3a2ee9 (diff)
downloadotp-c7cc7877e048eab4ee16169653d7cec51fe8a8df.tar.gz
otp-c7cc7877e048eab4ee16169653d7cec51fe8a8df.tar.bz2
otp-c7cc7877e048eab4ee16169653d7cec51fe8a8df.zip
hipe: Fix crashing bugs when inlining FP ops
It was assumed in hipe_icode_fp:filter_map/3 that either all predecessors would have an up-to-date ("assigned") tagged copy, or none of them. This is temporarily false during the fixpoint loop in basic_floats:test_icode_type_crash_2/0, which exercises the all_args_equal(Bindings) =:= true branch, that would previously erroneously end up in the 'false' branch, which is what caused the crash in that case. This is likewise only temporarily false during the fixpoint loop in basic_floats:test_icode_type_crash/0, but that case instead exercises the 'false' branch, justifying the inclusion of both tests.
Diffstat (limited to 'lib/hipe')
-rw-r--r--lib/hipe/icode/hipe_icode_fp.erl43
-rw-r--r--lib/hipe/test/basic_SUITE_data/basic_floats.erl41
2 files changed, 64 insertions, 20 deletions
diff --git a/lib/hipe/icode/hipe_icode_fp.erl b/lib/hipe/icode/hipe_icode_fp.erl
index 5ae0395b72..3e4a2f039a 100644
--- a/lib/hipe/icode/hipe_icode_fp.erl
+++ b/lib/hipe/icode/hipe_icode_fp.erl
@@ -738,8 +738,16 @@ filter_map([{Var, Bindings}|Left], NofPreds, Map) ->
true ->
case all_args_equal(Bindings) of
true ->
- {_, FVar} = hd(Bindings),
- filter_map(Left, NofPreds, gb_trees:update(Var, FVar, Map));
+ NewBinding =
+ case hd(Bindings) of
+ {Pred, {assigned, FVar0}} when is_integer(Pred) ->
+ case bindings_all_assigned(Bindings) of
+ true -> {assigned, FVar0};
+ false -> FVar0
+ end;
+ {Pred, FVar0} when is_integer(Pred) -> FVar0
+ end,
+ filter_map(Left, NofPreds, gb_trees:update(Var, NewBinding, Map));
false ->
PhiDst = hipe_icode:mk_new_fvar(),
PhiArgs = strip_of_assigned(Bindings),
@@ -751,7 +759,7 @@ filter_map([{Var, Bindings}|Left], NofPreds, Map) ->
gb_trees:update(phi, [{PhiDst, PhiArgs}|Val], Map)
end,
NewBinding =
- case bindings_are_assigned(Bindings) of
+ case bindings_all_assigned(Bindings) of
true -> {assigned, PhiDst};
false -> PhiDst
end,
@@ -763,30 +771,25 @@ filter_map([{Var, Bindings}|Left], NofPreds, Map) ->
filter_map([], _NofPreds, Map) ->
Map.
-bindings_are_assigned([{_, {assigned, _}}|Left]) ->
- assert_assigned(Left),
- true;
-bindings_are_assigned(Bindings) ->
- assert_not_assigned(Bindings),
- false.
-
-assert_assigned([{_, {assigned, _}}|Left]) ->
- assert_assigned(Left);
-assert_assigned([]) ->
- ok.
-
-assert_not_assigned([{_, FVar}|Left]) ->
- true = hipe_icode:is_fvar(FVar),
- assert_not_assigned(Left);
-assert_not_assigned([]) ->
- ok.
+bindings_all_assigned([]) -> true;
+bindings_all_assigned([{_, {assigned, _}}|Left]) ->
+ bindings_all_assigned(Left);
+bindings_all_assigned(_) -> false.
%% all_args_equal returns true if the mapping for a variable is the
%% same from all predecessors, i.e., we do not need a phi-node.
+%% During the fixpoint loop, a mapping might become assigned, without that
+%% information having propagated into all predecessors. We take care to answer
+%% true even if FVar is only assigned in some predecessors.
+
+all_args_equal([{_, {assigned, FVar}}|Left]) ->
+ all_args_equal(Left, FVar);
all_args_equal([{_, FVar}|Left]) ->
all_args_equal(Left, FVar).
+all_args_equal([{_, {assigned, FVar1}}|Left], FVar1) ->
+ all_args_equal(Left, FVar1);
all_args_equal([{_, FVar1}|Left], FVar1) ->
all_args_equal(Left, FVar1);
all_args_equal([], _) ->
diff --git a/lib/hipe/test/basic_SUITE_data/basic_floats.erl b/lib/hipe/test/basic_SUITE_data/basic_floats.erl
index 32bbb6c7fc..ebd3852002 100644
--- a/lib/hipe/test/basic_SUITE_data/basic_floats.erl
+++ b/lib/hipe/test/basic_SUITE_data/basic_floats.erl
@@ -19,6 +19,7 @@ test() ->
ok = test_fp_with_fp_exceptions(),
%% ok = test_fmt_double_fpe_leak(), % this requires printing
ok = test_icode_type_crash(),
+ ok = test_icode_type_crash_2(),
ok.
%%--------------------------------------------------------------------
@@ -208,3 +209,43 @@ f(A, B, C) ->
end,
Y * Z
end.
+
+%%%-------------------------------------------------------------------
+%%% Contains another case that crashes hipe_icode_fp. This sample was
+%%% sent by Mattias Jansson. Compiles alright with the option
+%%% 'no_icode_type' but that defeats the purpose of the test.
+
+test_icode_type_crash_2() ->
+ {'EXIT', {function_clause, _}} = (catch eat()),
+ ok.
+
+eat() ->
+ eat_what(1.0, #{}).
+
+eat_what(Factor, #{rat_type := LT} = Rat) ->
+ #{ cheese := Cheese } = Rat,
+ UnitCheese = Cheese / 2,
+ RetA = case eat() of
+ {full, RetA1} ->
+ CheeseB2 = min(RetA1, UnitCheese) * Factor,
+ case eat() of
+ full ->
+ {win, RetA1};
+ hungry ->
+ {partial, RetA1 - CheeseB2}
+ end;
+ AOther ->
+ AOther
+ end,
+ RetB = case eat() of
+ {full, RetB1} ->
+ CheeseA2 = min(RetB1, UnitCheese) * Factor,
+ rat:init(single, LT, CheeseA2),
+ case eat() of
+ full ->
+ {full, RetB1};
+ hungry ->
+ {hungry, RetB1 - CheeseA2}
+ end
+ end,
+ {RetA, RetB}.