From 7d06e5ecd47ef071eb8c4e58ead70e79fa4b02b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 31 Mar 2016 06:55:31 +0200
Subject: Fix unsafe transformation of apply/3 with fixed arguments

62473daf introduced an unsafe optimization in the loader.
See the comments in the test case for an explanation of
the problem.
---
 erts/emulator/beam/beam_load.c    |  7 ++++++
 erts/emulator/beam/ops.tab        |  2 +-
 erts/emulator/test/beam_SUITE.erl | 45 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 50 insertions(+), 4 deletions(-)

(limited to 'erts')

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index d3d278fb81..16cbdbffea 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -2734,6 +2734,13 @@ same_label(LoaderState* stp, GenOpArg Target, GenOpArg Label)
 	Target.val == Label.val;
 }
 
+static int
+is_killed_apply(LoaderState* stp, GenOpArg Reg, GenOpArg Live)
+{
+    return Reg.type == TAG_x && Live.type == TAG_u &&
+	Live.val+2 <= Reg.val;
+}
+
 static int
 is_killed(LoaderState* stp, GenOpArg Reg, GenOpArg Live)
 {
diff --git a/erts/emulator/beam/ops.tab b/erts/emulator/beam/ops.tab
index 9e53b4bfcc..772460c177 100644
--- a/erts/emulator/beam/ops.tab
+++ b/erts/emulator/beam/ops.tab
@@ -303,7 +303,7 @@ move_window5 x x x x x y
 # Swap registers.
 move R1=x Tmp=x | move R2=xy R1 | move Tmp R2 => swap_temp R1 R2 Tmp
 
-swap_temp R1 R2 Tmp | line Loc | apply Live | is_killed(Tmp, Live) => \
+swap_temp R1 R2 Tmp | line Loc | apply Live | is_killed_apply(Tmp, Live) => \
   swap R1 R2 | line Loc | apply Live
 
 swap_temp R1 R2 Tmp | line Loc | call Live Addr | is_killed(Tmp, Live) => \
diff --git a/erts/emulator/test/beam_SUITE.erl b/erts/emulator/test/beam_SUITE.erl
index 07dfeb6633..f61ab431e9 100644
--- a/erts/emulator/test/beam_SUITE.erl
+++ b/erts/emulator/test/beam_SUITE.erl
@@ -24,9 +24,9 @@
 	 init_per_group/2,end_per_group/2, 
 	 packed_registers/1, apply_last/1, apply_last_bif/1,
 	 buildo_mucho/1, heap_sizes/1, big_lists/1, fconv/1,
-	 select_val/1]).
+	 select_val/1, swap_temp_apply/1]).
 
--export([applied/2]).
+-export([applied/2,swap_temp_applied/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
@@ -34,7 +34,8 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
     [packed_registers, apply_last, apply_last_bif,
-     buildo_mucho, heap_sizes, big_lists, select_val].
+     buildo_mucho, heap_sizes, big_lists, select_val,
+     swap_temp_apply].
 
 groups() -> 
     [].
@@ -346,3 +347,41 @@ do_select_val(X) ->
 	Int when is_integer(Int) ->
 	    integer
     end.
+
+swap_temp_apply(_Config) ->
+    {swap_temp_applied,42} = do_swap_temp_apply(41),
+    not_an_integer = do_swap_temp_apply(not_an_integer),
+    ok.
+
+do_swap_temp_apply(Msg) ->
+    case swap_temp_apply_function(Msg) of
+	undefined -> Msg;
+	Type ->
+	    %% The following sequence:
+	    %%   move {x,0} {x,2}
+	    %%   move {y,0} {x,0}
+	    %%   move {x,2} {y,0}
+	    %%   apply 1
+	    %%
+	    %% Would be incorrectly transformed to:
+	    %%   swap {x,0} {y,0}
+	    %%   apply 1
+	    %%
+	    %% ({x,1} is the module, {x,2} the function to be applied).
+	    %%
+	    %% If the instructions are to be transformed, the correct
+	    %% transformation is:
+	    %%
+	    %%   swap_temp {x,0} {y,0} {x,2}
+	    %%   apply 1
+	    Fields = ?MODULE:Type(Msg),
+	    {Type,Fields}
+    end.
+
+swap_temp_apply_function(Int) when is_integer(Int) ->
+    swap_temp_applied;
+swap_temp_apply_function(_) ->
+    undefined.
+
+swap_temp_applied(Int) ->
+    Int+1.
-- 
cgit v1.2.3