beam_block: Eliminate unsafe optimization

Consider this code:

    %% Start of block
    get_tuple_element Tuple 0 Element
    get_map_elements Fail Map [Key => Dest]
    .
    .
    .
    move Element UltimateDest
    %% End of block

  Fail:
    %% Code that uses Element.

beam_block (more precisely, otp_tuple_element/1) would
incorrectly transform the code to this:

    %% Start of block
    get_map_elements Fail Map [Key => Dest]
    .
    .
    .
    get_tuple_element Tuple 0 UltimateDest
    %% End of block

  Fail:
    %% Code that uses Element.

That is, the code at label Fail would use register Element,
which is either uninitalized or contains the wrong value.

We could fix this problem by always keeping label information
at hand when optimizing blocks so that we could check the code
at the failure label for get_map_elements. That would require
changes to beam_block and beam_utils. We might consider doing
that in the future if it turns out be worth it.

For now, I have decided that I want to keep the simplicity of blocks
(allowing them to be optimized without keeping label information).
That could be achieved by not including get_map_elements in
blocks. Another way, which I have chosen, is to only allow
get_map_elements as the first instruction in the block.

For background on the bug: c288ab8 introduced the beam_reorder pass
and 5f431276 introduced opt_tuple_element() in beam_block.

3 files changed, 76 insertions, 3 deletions

diff --git a/lib/compiler/src/beam_block.erl b/lib/compiler/src/beam_block.erl
index 10dbaf462c..d1052303e0 100644
--- a/lib/compiler/src/beam_block.erl
+++ b/lib/compiler/src/beam_block.erl
@@ -58,6 +58,13 @@ blockify(Is) ->
 blockify([{loop_rec,{f,Fail},{x,0}},{loop_rec_end,_Lbl},{label,Fail}|Is], Acc) ->
     %% Useless instruction sequence.
     blockify(Is, Acc);
+blockify([{get_map_elements,F,S,{list,Gets}}|Is0], Acc) ->
+    %% A get_map_elements instruction is only safe at the beginning of
+    %% a block because of the failure label.
+    {Ss,Ds} = beam_utils:split_even(Gets),
+    I = {set,Ds,[S|Ss],{get_map_elements,F}},
+    {Block,Is} = collect_block(Is0, [I]),
+    blockify(Is, [{block,Block}|Acc]);
 blockify([I|Is0]=IsAll, Acc) ->
     case collect(I) of
 	error -> blockify(Is0, [I|Acc]);
@@ -101,9 +108,6 @@ collect({get_list,S,D1,D2})  -> {set,[D1,D2],[S],get_list};
 collect(remove_message)      -> {set,[],[],remove_message};
 collect({put_map,F,Op,S,D,R,{list,Puts}}) ->
     {set,[D],[S|Puts],{alloc,R,{put_map,Op,F}}};
-collect({get_map_elements,F,S,{list,Gets}}) ->
-    {Ss,Ds} = beam_utils:split_even(Gets),
-    {set,Ds,[S|Ss],{get_map_elements,F}};
 collect({'catch'=Op,R,L}) ->
     {set,[R],[],{try_catch,Op,L}};
 collect({'try'=Op,R,L}) ->
diff --git a/lib/compiler/test/Makefile b/lib/compiler/test/Makefile
index c2d757da4d..85118502e3 100644
--- a/lib/compiler/test/Makefile
+++ b/lib/compiler/test/Makefile
@@ -8,6 +8,7 @@ include $(ERL_TOP)/make/$(TARGET)/otp.mk
 MODULES= \
 	andor_SUITE \
 	apply_SUITE \
+	beam_block_SUITE \
 	beam_validator_SUITE \
 	beam_disasm_SUITE \
 	beam_except_SUITE \
@@ -44,6 +45,7 @@ MODULES= \
 NO_OPT= \
 	andor \
 	apply \
+	beam_block \
 	beam_except \
 	beam_reorder \
 	beam_type \
@@ -67,6 +69,7 @@ NO_OPT= \
 INLINE= \
 	andor \
 	apply \
+	beam_block \
 	beam_utils \
 	bs_bincomp \
 	bs_bit_binaries \
diff --git a/lib/compiler/test/beam_block_SUITE.erl b/lib/compiler/test/beam_block_SUITE.erl
new file mode 100644
index 0000000000..7b6f2066be
--- /dev/null
+++ b/lib/compiler/test/beam_block_SUITE.erl
@@ -0,0 +1,66 @@
+%%
+%% %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_block_SUITE).
+
+-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]).
+
+suite() -> [{ct_hooks,[ts_install_cth]}].
+
+all() ->
+    test_lib:recompile(?MODULE),
+    [{group,p}].
+
+groups() ->
+    [{p,[parallel],
+      [get_map_elements
+      ]}].
+
+init_per_suite(Config) ->
+    Config.
+
+end_per_suite(_Config) ->
+    ok.
+
+init_per_group(_GroupName, Config) ->
+    Config.
+
+end_per_group(_GroupName, Config) ->
+    Config.
+
+get_map_elements(_Config) ->
+    [{pred,var}] = get_map_elements([{pred,var}], #{}, []),
+    [{pred,var}] = get_map_elements([{pred,var}], #{pred=>[]}, []),
+    acc = get_map_elements([], #{pred=>[]}, acc),
+    ok.
+
+get_map_elements([{Pred,Var}|Left], Map, Acc) ->
+    case Map of
+        #{Var := List} ->
+            case lists:keyfind(Pred, 1, List) of
+                false ->
+                    get_map_elements(Left, Map, [{Pred,Var}|Acc])
+            end;
+        #{} ->
+            get_map_elements(Left, Map, [{Pred,Var}|Acc])
+    end;
+get_map_elements([], _Map, Acc) ->
+    Acc.