From 0c599bcad1e7f5f66dd2342ab27791048145e892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Mon, 26 Sep 2016 15:01:19 +0200 Subject: beam_block: Avoid unsafe inclusion of get_map_elements in blocks c2035ebb8b restricted the get_map_elements instruction so that it could only occur at the beginning of a block. It turns out that including it anywhere in a block is unsafe. Therefore, never put get_map_elements instruction in blocks. (Also remove the beam_utils:join_even/2 function since it is no longer used.) ERL-266 --- lib/compiler/src/beam_block.erl | 10 ---------- lib/compiler/src/beam_flatten.erl | 1 - lib/compiler/src/beam_split.erl | 3 --- lib/compiler/src/beam_utils.erl | 12 ++++++------ lib/compiler/test/beam_block_SUITE.erl | 26 ++++++++++++++++++++++++-- 5 files changed, 30 insertions(+), 22 deletions(-) (limited to 'lib/compiler') diff --git a/lib/compiler/src/beam_block.erl b/lib/compiler/src/beam_block.erl index ec41925beb..6a35191f6e 100644 --- a/lib/compiler/src/beam_block.erl +++ b/lib/compiler/src/beam_block.erl @@ -58,13 +58,6 @@ 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]); @@ -220,7 +213,6 @@ move_allocates_1([], Acc) -> Acc. alloc_may_pass({set,_,_,{alloc,_,_}}) -> false; alloc_may_pass({set,_,_,{set_tuple_element,_}}) -> false; -alloc_may_pass({set,_,_,{get_map_elements,_}}) -> false; alloc_may_pass({set,_,_,put_list}) -> false; alloc_may_pass({set,_,_,put}) -> false; alloc_may_pass({set,_,_,_}) -> true. @@ -235,8 +227,6 @@ opt([{set,_,_,{line,_}}=Line1, {set,[D2],[{integer,Idx2},Reg],{bif,element,{f,0}}}=I2|Is]) when Idx1 < Idx2, D1 =/= D2, D1 =/= Reg, D2 =/= Reg -> opt([Line2,I2,Line1,I1|Is]); -opt([{set,[_|_],_Ss,{get_map_elements,_F}}=I|Is]) -> - [I|opt(Is)]; opt([{set,Ds0,Ss,Op}|Is0]) -> {Ds,Is} = opt_moves(Ds0, Is0), [{set,Ds,Ss,Op}|opt(Is)]; diff --git a/lib/compiler/src/beam_flatten.erl b/lib/compiler/src/beam_flatten.erl index 36369bd0b4..c9ff07b496 100644 --- a/lib/compiler/src/beam_flatten.erl +++ b/lib/compiler/src/beam_flatten.erl @@ -64,7 +64,6 @@ norm({set,[],[S,D],{set_tuple_element,I}}) -> {set_tuple_element,S,D,I}; norm({set,[D1,D2],[S],get_list}) -> {get_list,S,D1,D2}; norm({set,[D],[S|Puts],{alloc,R,{put_map,Op,F}}}) -> {put_map,F,Op,S,D,R,{list,Puts}}; -%% get_map_elements is always handled in beam_split (moved out of block) norm({set,[],[],remove_message}) -> remove_message; norm({set,[],[],fclearerror}) -> fclearerror; norm({set,[],[],fcheckerror}) -> {fcheckerror,{f,0}}. diff --git a/lib/compiler/src/beam_split.erl b/lib/compiler/src/beam_split.erl index c83c686953..feeab0af50 100644 --- a/lib/compiler/src/beam_split.erl +++ b/lib/compiler/src/beam_split.erl @@ -56,9 +56,6 @@ split_block([{set,[D],[S|Puts],{alloc,R,{put_map,Op,{f,Lbl}=Fail}}}|Is], Bl, Acc) when Lbl =/= 0 -> split_block(Is, [], [{put_map,Fail,Op,S,D,R,{list,Puts}}| make_block(Bl, Acc)]); -split_block([{set,Ds,[S|Ss],{get_map_elements,Fail}}|Is], Bl, Acc) -> - Gets = beam_utils:join_even(Ss,Ds), - split_block(Is, [], [{get_map_elements,Fail,S,{list,Gets}}|make_block(Bl, Acc)]); split_block([{set,[R],[],{try_catch,Op,L}}|Is], Bl, Acc) -> split_block(Is, [], [{Op,R,L}|make_block(Bl, Acc)]); split_block([{set,[],[],{line,_}=Line}|Is], Bl, Acc) -> diff --git a/lib/compiler/src/beam_utils.erl b/lib/compiler/src/beam_utils.erl index a15ecf633e..249d9395ca 100644 --- a/lib/compiler/src/beam_utils.erl +++ b/lib/compiler/src/beam_utils.erl @@ -26,7 +26,7 @@ empty_label_index/0,index_label/3,index_labels/1, code_at/2,bif_to_test/3,is_pure_test/1, live_opt/1,delete_live_annos/1,combine_heap_needs/2, - join_even/2,split_even/1]). + split_even/1]). -import(lists, [member/2,sort/1,reverse/1,splitwith/2]). @@ -233,11 +233,6 @@ combine_heap_needs(H1, H2) when is_integer(H1), is_integer(H2) -> split_even(Rs) -> split_even(Rs, [], []). -%% join_even/1 -%% {[1,3,5],[2,4,6]} -> [1,2,3,4,5,6] - -join_even([], []) -> []; -join_even([S|Ss], [D|Ds]) -> [S,D|join_even(Ss, Ds)]. %%% %%% Local functions. @@ -753,6 +748,11 @@ live_opt([timeout=I|Is], _, D, Acc) -> live_opt(Is, 0, D, [I|Acc]); live_opt([{wait,_}=I|Is], _, D, Acc) -> live_opt(Is, 0, D, [I|Acc]); +live_opt([{get_map_elements,Fail,Src,{list,List}}=I|Is], Regs0, D, Acc) -> + {Ss,Ds} = split_even(List), + Regs1 = x_live([Src|Ss], x_dead(Ds, Regs0)), + Regs = live_join_label(Fail, D, Regs1), + live_opt(Is, Regs, D, [I|Acc]); %% Transparent instructions - they neither use nor modify x registers. live_opt([{deallocate,_}=I|Is], Regs, D, Acc) -> diff --git a/lib/compiler/test/beam_block_SUITE.erl b/lib/compiler/test/beam_block_SUITE.erl index 9fcb6e497d..55d5f2dbe8 100644 --- a/lib/compiler/test/beam_block_SUITE.erl +++ b/lib/compiler/test/beam_block_SUITE.erl @@ -22,7 +22,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,move_opt_across_gc_bif/1, - erl_202/1]). + erl_202/1,repro/1]). %% The only test for the following functions is that %% the code compiles and is accepted by beam_validator. @@ -39,7 +39,8 @@ groups() -> [get_map_elements, otp_7345, move_opt_across_gc_bif, - erl_202 + erl_202, + repro ]}]. init_per_suite(Config) -> @@ -158,6 +159,27 @@ erl_202({{_, _},X}, _) -> erl_202({_, _}, #erl_202_r1{y=R2}) -> {R2#erl_202_r2.x}. +%% See https://bugs.erlang.org/browse/ERL-266. +%% Instructions with failure labels are not safe to include +%% in a block. Including get_map_elements in a block would +%% lead to unsafe code. + +repro(_Config) -> + [] = maps:to_list(repro([], #{}, #{})), + [{tmp1,n}] = maps:to_list(repro([{tmp1,0}], #{}, #{})), + [{tmp1,name}] = maps:to_list(repro([{tmp1,0}], #{}, #{0=>name})), + ok. + +repro([], TempNames, _Slots) -> + TempNames; +repro([{Temp, Slot}|Xs], TempNames, Slots0) -> + {Name, Slots} = + case Slots0 of + #{Slot := Name0} -> {Name0, Slots0}; + #{} -> {n, Slots0#{Slot => n}} + end, + repro(Xs, TempNames#{Temp => Name}, Slots). + %%% %%% The only test of the following code is that it compiles. %%% -- cgit v1.2.3