From c7cc7877e048eab4ee16169653d7cec51fe8a8df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Magnus=20L=C3=A5ng?= <margnus1@telia.com>
Date: Wed, 2 Mar 2016 14:55:25 +0100
Subject: 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.
---
 lib/hipe/icode/hipe_icode_fp.erl                | 43 +++++++++++++------------
 lib/hipe/test/basic_SUITE_data/basic_floats.erl | 41 +++++++++++++++++++++++
 2 files changed, 64 insertions(+), 20 deletions(-)

(limited to 'lib/hipe')

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}.
-- 
cgit v1.2.3