From aab1db16d0a732823fa9e2964c6a52b109c61742 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Tue, 31 May 2016 22:49:42 +0200
Subject: beam_block: Eliminate crash in beam_utils

Somewhat simplified, beam_block would rewrite the target for
the first instruction in this code sequence:

    move x(0) => y(1)
    gc_bif '+' 1 x(0) => y(0)
    move y(1) => x(1)
    move nil => x(0)
    call 2 local_function/2

The resulting code would be:

    move x(0) => x(1)           %% Changed target.
    gc_bif '+' 1 x(0) => y(0)
    move x(1) => y(1)           %% Operands swapped (see 02d6135813).
    move nil => x(0)
    call 2 local_function/2

The resulting code is not safe because the x(1) will be killed
by the gc_bif instruction.

7a47b20c3a cleaned up move optimizations and would reject the
optimization if the target was an X register and an allocating
instruction was found. To avoid this bug, the optimization must be
rejected even if the target is a Y register.
---
 lib/compiler/src/beam_block.erl        | 17 +++++++++++------
 lib/compiler/test/beam_block_SUITE.erl | 21 +++++++++++++++++++--
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/lib/compiler/src/beam_block.erl b/lib/compiler/src/beam_block.erl
index a8cfdffdf3..85d332c56e 100644
--- a/lib/compiler/src/beam_block.erl
+++ b/lib/compiler/src/beam_block.erl
@@ -262,12 +262,17 @@ opt_move_1(R, [{set,[D],[R],move}|Is0], Acc) ->
 	{yes,Is} -> opt_move_rev(D, Acc, Is);
 	no -> not_possible
     end;
-opt_move_1({x,_}, [{set,_,_,{alloc,_,_}}|_], _) ->
-    %% The optimization is not possible. If the X register is not
-    %% killed by allocation, the optimization would not be safe.
-    %% If the X register is killed, it means that there cannot
-    %% follow a 'move' instruction with this X register as the
-    %% source.
+opt_move_1(_R, [{set,_,_,{alloc,_,_}}|_], _) ->
+    %% The optimization is either not possible or not safe.
+    %%
+    %% If R is an X register killed by allocation, the optimization is
+    %% not safe. On the other hand, if the X register is killed, there
+    %% will not follow a 'move' instruction with this X register as
+    %% the source.
+    %%
+    %% If R is a Y register, the optimization is still not safe
+    %% because the new target register is an X register that cannot
+    %% safely pass the alloc instruction.
     not_possible;
 opt_move_1(R, [{set,_,_,_}=I|Is], Acc) ->
     %% If the source register is either killed or used by this
diff --git a/lib/compiler/test/beam_block_SUITE.erl b/lib/compiler/test/beam_block_SUITE.erl
index d343e26737..4bcb252833 100644
--- a/lib/compiler/test/beam_block_SUITE.erl
+++ b/lib/compiler/test/beam_block_SUITE.erl
@@ -21,7 +21,7 @@
 
 -export([all/0,suite/0,groups/0,init_per_suite/1,end_per_suite/1,
 	 init_per_group/2,end_per_group/2,
-	 get_map_elements/1,otp_7345/1]).
+	 get_map_elements/1,otp_7345/1,move_opt_across_gc_bif/1]).
 
 %% The only test for the following functions is that
 %% the code compiles and is accepted by beam_validator.
@@ -36,7 +36,8 @@ all() ->
 groups() ->
     [{p,[parallel],
       [get_map_elements,
-       otp_7345
+       otp_7345,
+       move_opt_across_gc_bif
       ]}].
 
 init_per_suite(Config) ->
@@ -118,6 +119,22 @@ otp_7345(ObjRef, _RdEnv, Args) ->
 		       10},
     id(LlUnitdataReq).
 
+
+%% Doing move optimizations across GC bifs are in general not safe.
+move_opt_across_gc_bif(_Config) ->
+    [0,true,1] = positive(speaking),
+    ok.
+
+positive(speaking) ->
+    try
+	Positive = 0,
+	[+Positive, case Positive of _ -> true end, paris([], Positive)]
+    after
+	mailing
+    end.
+
+paris([], P) -> P + 1.
+
 %%%
 %%% The only test of the following code is that it compiles.
 %%%
-- 
cgit v1.2.3