From 76bb9f0e9a0b36ea7c9720c2bf90f6b52a4eabf8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Mon, 5 Oct 2015 06:43:12 +0200
Subject: beam_reorder: Eliminate compiler crash

c288ab87 added beam_reorder to move get_tuple_element instructions.

Compiling code such as the following would crash the compiler:

  alloc(_U1, _U2, R) ->
    V = R#alloc.version,
    Res = id(V),
    _ = id(0),
    Res.

The crash would occur because the following two instructions:

    {get_tuple_element,{x,2},1,{x,1}}.
    {allocate_zero,1,2}.

were swapped and rewritten to:

    {allocate_zero,1,1}.
    {get_tuple_element,{x,2},1,{x,1}}.

That transformation is not safe because the allocate_zero instruction
would kill {x,2}, which is the register that is holding the reference
to the tuple. Only do the transformation when the tuple reference is
in an x register with a lower number than the destination register.
---
 lib/compiler/src/beam_reorder.erl        | 21 ++++++----
 lib/compiler/test/Makefile               |  2 +
 lib/compiler/test/beam_reorder_SUITE.erl | 69 ++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 8 deletions(-)
 create mode 100644 lib/compiler/test/beam_reorder_SUITE.erl

diff --git a/lib/compiler/src/beam_reorder.erl b/lib/compiler/src/beam_reorder.erl
index 70adca6b04..41586a7bf2 100644
--- a/lib/compiler/src/beam_reorder.erl
+++ b/lib/compiler/src/beam_reorder.erl
@@ -110,14 +110,19 @@ reorder_1([{test,_,{f,L},Ss}=I|Is0], D0,
 		    end
 	    end
     end;
-reorder_1([{allocate_zero,N,Live}|Is], D,
-	  [{get_tuple_element,_,_,{x,X}}=G|Acc])
-  when X+1 =:= Live ->
-    %% Move allocation instruction upwards past get_tuple_element
-    %% instructions to give more opportunities for moving
-    %% get_tuple_element instructions.
-    I = {allocate_zero,N,X},
-    reorder_1([I,G|Is], D, Acc);
+reorder_1([{allocate_zero,N,Live}=I0|Is], D,
+	  [{get_tuple_element,{x,Tup},_,{x,Dst}}=G|Acc]=Acc0) ->
+    case Tup < Dst andalso Dst+1 =:= Live of
+	true ->
+	    %% Move allocation instruction upwards past
+	    %% get_tuple_element instructions to create more
+	    %% opportunities for moving get_tuple_element
+	    %% instructions.
+	    I = {allocate_zero,N,Dst},
+	    reorder_1([I,G|Is], D, Acc);
+	false ->
+	    reorder_1(Is, D, [I0|Acc0])
+    end;
 reorder_1([I|Is], D, Acc) ->
     reorder_1(Is, D, [I|Acc]);
 reorder_1([], _, Acc) -> reverse(Acc).
diff --git a/lib/compiler/test/Makefile b/lib/compiler/test/Makefile
index 0cd8618730..400565100f 100644
--- a/lib/compiler/test/Makefile
+++ b/lib/compiler/test/Makefile
@@ -11,6 +11,7 @@ MODULES= \
 	beam_validator_SUITE \
 	beam_disasm_SUITE \
 	beam_except_SUITE \
+	beam_reorder_SUITE \
 	beam_type_SUITE \
 	beam_utils_SUITE \
 	bs_bincomp_SUITE \
@@ -44,6 +45,7 @@ NO_OPT= \
 	andor \
 	apply \
 	beam_except \
+	beam_reorder \
 	beam_type \
 	beam_utils \
 	bs_construct \
diff --git a/lib/compiler/test/beam_reorder_SUITE.erl b/lib/compiler/test/beam_reorder_SUITE.erl
new file mode 100644
index 0000000000..4b2262f65b
--- /dev/null
+++ b/lib/compiler/test/beam_reorder_SUITE.erl
@@ -0,0 +1,69 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2015. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%
+%% %CopyrightEnd%
+%%
+-module(beam_reorder_SUITE).
+
+-export([all/0,suite/0,groups/0,init_per_suite/1,end_per_suite/1,
+	 init_per_group/2,end_per_group/2,
+	 alloc/1]).
+
+suite() -> [{ct_hooks,[ts_install_cth]}].
+
+all() ->
+    test_lib:recompile(?MODULE),
+    [{group,p}].
+
+groups() ->
+    [{p,[parallel],
+      [alloc
+      ]}].
+
+init_per_suite(Config) ->
+    Config.
+
+end_per_suite(_Config) ->
+    ok.
+
+init_per_group(_GroupName, Config) ->
+    Config.
+
+end_per_group(_GroupName, Config) ->
+    Config.
+
+-record(alloc, {version}).
+
+alloc(_Config) ->
+    {ok,42} = alloc_a(1, 2, #alloc{version=42}),
+    {a,b,c} = alloc_b(1, 2, #alloc{version={a,b,c}}),
+    ok.
+
+alloc_a(_U1, _U2, R) ->
+    V = R#alloc.version,
+    Res = id({ok,V}),
+    _ = id(0),
+    Res.
+
+alloc_b(_U1, _U2, R) ->
+    V = R#alloc.version,
+    Res = id(V),
+    _ = id(0),
+    Res.
+
+id(I) ->
+    I.
-- 
cgit v1.2.3