From 58d6a866dae481a098ea583439cac3fbb95df787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Sun, 21 Oct 2018 09:27:59 +0200
Subject: Make the move elimination optimization in beam_jump safe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `beam_jump` pass could eliminate `move` instructions when it was
not safe to do so. See the new test case `unsafe_move_elimination/1`
for an example.

Reported-by: Michał Muskała
---
 lib/compiler/src/beam_jump.erl        | 13 ++++------
 lib/compiler/test/beam_jump_SUITE.erl | 47 +++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/lib/compiler/src/beam_jump.erl b/lib/compiler/src/beam_jump.erl
index fbff4cfd79..d694bd2a5b 100644
--- a/lib/compiler/src/beam_jump.erl
+++ b/lib/compiler/src/beam_jump.erl
@@ -196,22 +196,19 @@ no_fallthrough([I|_]) ->
     is_unreachable_after(I).
 
 already_has_value(Lit, Lbl, Reg, D) ->
-    Key = {Lbl,Reg},
     case D of
-        #{Lbl:=unsafe} ->
-            false;
-        #{Key:=Lit} ->
+        #{Lbl:={Reg,Lit}} ->
             true;
         #{} ->
             false
     end.
 
 update_value_dict([Lit,{f,Lbl}|T], Reg, D0) ->
-    Key = {Lbl,Reg},
     D = case D0 of
-            #{Key := inconsistent} -> D0;
-            #{Key := _} -> D0#{Key := inconsistent};
-            _ -> D0#{Key => Lit}
+            #{Lbl:=unsafe} -> D0;
+            #{Lbl:={Reg,Lit}} -> D0;
+            #{Lbl:=_} -> D0#{Lbl:=unsafe};
+            #{} -> D0#{Lbl=>{Reg,Lit}}
         end,
     update_value_dict(T, Reg, D);
 update_value_dict([], _, D) -> D.
diff --git a/lib/compiler/test/beam_jump_SUITE.erl b/lib/compiler/test/beam_jump_SUITE.erl
index c61e4ab65c..f6cddbf1b1 100644
--- a/lib/compiler/test/beam_jump_SUITE.erl
+++ b/lib/compiler/test/beam_jump_SUITE.erl
@@ -21,7 +21,8 @@
 
 -export([all/0,suite/0,groups/0,init_per_suite/1,end_per_suite/1,
 	 init_per_group/2,end_per_group/2,
-	 undefined_label/1,ambiguous_catch_try_state/1]).
+	 undefined_label/1,ambiguous_catch_try_state/1,
+         unsafe_move_elimination/1]).
 
 suite() ->
     [{ct_hooks,[ts_install_cth]}].
@@ -32,7 +33,8 @@ all() ->
 groups() ->
     [{p,[parallel],
       [undefined_label,
-       ambiguous_catch_try_state
+       ambiguous_catch_try_state,
+       unsafe_move_elimination
       ]}].
 
 init_per_suite(Config) ->
@@ -72,3 +74,44 @@ river() -> song.
 checks(Wanted) ->
     %% Must be one line to cause the unsafe optimization.
     {catch case river() of sheet -> begin +Wanted, if "da" -> Wanted end end end, catch case river() of sheet -> begin + Wanted, if "da" -> Wanted end end end}.
+
+unsafe_move_elimination(_Config) ->
+    {{left,right,false},false} = unsafe_move_elimination(left, right, false),
+    {{false,right,false},false} = unsafe_move_elimination(false, right, true),
+    {{true,right,right},right} = unsafe_move_elimination(true, right, true),
+    ok.
+
+unsafe_move_elimination(Left, Right, Simple0) ->
+    id(1),
+
+    %% The move at label 29 would be removed by beam_jump, which is unsafe because
+    %% the two select_val instructions have different source registers.
+    %%
+    %%   {select_val,{y,0},{f,25},{list,[{atom,true},{f,27},{atom,false},{f,29}]}}.
+    %%               ^^^^^                                  ^^^^^^^^^^^^^^^^^^^
+    %% {label,27}.
+    %%   {kill,{y,0}}.
+    %%   {move,{y,2},{x,0}}.
+    %%   {line,...}.
+    %%   {call,1,{f,31}}.
+    %%   {select_val,{x,0},{f,33},{list,[{atom,true},{f,35},{atom,false},{f,29}]}}.
+    %%               ^^^^^                                  ^^^^^^^^^^^^^^^^^^^
+    %% {label,29}.
+    %%   {move,{atom,false},{y,0}}.  <=== REMOVED (unsafely).
+    %%   {jump,{f,37}}.
+
+    Simple = case case Simple0 of
+                      false -> false;
+                      true -> id(Left)
+                  end
+             of
+                 false ->
+                     false;
+                 true ->
+                     id(Right)
+             end,
+    {id({Left,Right,Simple}),Simple}.
+
+
+id(I) ->
+    I.
-- 
cgit v1.2.3