From aab1db16d0a732823fa9e2964c6a52b109c61742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= 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 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'lib/compiler/src') 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 -- cgit v1.2.3 From 5facb518a9ee2d756564eccd92a2c11f334d6282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 2 Jun 2016 09:58:46 +0200 Subject: Eliminate crash for map updates in guards beam_validator would complain that x(1) is uninitialized in a test_heap instruction when attempting to compile the following code with sys_core_fold turned off: foo(M) when not (M#{true := 0}); [M] -> ok. Simplified, the generated BEAM assembly code looked like this: test is_map BadMap x(0) put_map_exact Fail x(0) => x(1) ... jump BooleanStuff BadMap: move ok => x(1) jump Fail BooleanStuff: ... move Boolean => x(2) jump Build Fail: move false => x(2) Build: test_heap 2 3 %% x(0), x(1), x(2) must be live. ... That is, if put_map_exact failed, control would transfer to the label Fail without initializing x(1). Fix that by making sure that x(1) is initilized even if put_map_exact fails: test is_map BadMap x(0) put_map_exact BadLbl x(0) => x(1) ... jump OkLbl BadLbl: move ok => x(1) jump Fail OkLbl: jump BooleanStuff BadMap: move ok => x(1) jump Fail BooleanStuff: ... move Boolean => x(2) jump Build Fail: move false => x(2) Build: test_heap 2 3 %% x(0), x(1), x(2) must be live. ... Note that this situation is rare, and that other optimization passes (beam_dead and beam_jump in particular) will clean up this mess. --- lib/compiler/src/beam_validator.erl | 4 ++- lib/compiler/src/v3_codegen.erl | 60 ++++++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 21 deletions(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl index 13aa31b7c9..4c0cb6780a 100644 --- a/lib/compiler/src/beam_validator.erl +++ b/lib/compiler/src/beam_validator.erl @@ -658,8 +658,10 @@ valfun_4({test,is_map,{f,Lbl},[Src]}, Vst0) -> case Src of {Tag,_} when Tag =:= x; Tag =:= y -> set_type_reg(map, Src, Vst); + {literal,Map} when is_map(Map) -> + Vst; _ -> - Vst + kill_state(Vst) end; valfun_4({test,_Op,{f,Lbl},Src}, Vst) -> validate_src(Src, Vst), diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl index f5f3c73793..4df1aadd0a 100644 --- a/lib/compiler/src/v3_codegen.erl +++ b/lib/compiler/src/v3_codegen.erl @@ -1551,12 +1551,10 @@ set_cg([{var,R}], {binary,Segs}, Le, Vdb, Bef, #cg{bfail=Bfail}=St) -> %% Now generate the complete code for constructing the binary. Code = cg_binary(PutCode, Target, Temp, Fail, MaxRegs, Le#l.a), {Sis++Code,Aft,St}; -% Map single variable key -set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef, - #cg{bfail=Bfail}=St) -> - Fail = {f,Bfail}, - {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St), +%% Map: single variable key. +set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef, St0) -> + {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St0), SrcReg = cg_reg_arg_prefer_y(Map, Int0), Line = line(Le#l.a), @@ -1570,21 +1568,16 @@ set_cg([{var,R}], {map,Op,Map,[{map_pair,{var,_}=K,V}]}, Le, Vdb, Bef, Aft = Aft0#sr{reg=put_reg(R, Aft0#sr.reg)}, Target = fetch_reg(R, Aft#sr.reg), - I = case Op of - assoc -> put_map_assoc; - exact -> put_map_exact - end, - {Sis++[Line]++[{I,Fail,SrcReg,Target,Live,{list,List}}],Aft,St}; + {Is,St1} = set_cg_map(Line, Op, SrcReg, Target, Live, List, St0), + {Sis++Is,Aft,St1}; -% Map (possibly) multiple literal keys -set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef, - #cg{bfail=Bfail}=St) -> +%% Map: (possibly) multiple literal keys. +set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef, St0) -> %% assert key literals [] = [Var||{map_pair,{var,_}=Var,_} <- Es], - Fail = {f,Bfail}, - {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St), + {Sis,Int0} = maybe_adjust_stack(Bef, Le#l.i, Le#l.i+1, Vdb, St0), SrcReg = cg_reg_arg_prefer_y(Map, Int0), Line = line(Le#l.a), @@ -1599,11 +1592,10 @@ set_cg([{var,R}], {map,Op,Map,Es}, Le, Vdb, Bef, Aft = Aft0#sr{reg=put_reg(R, Aft0#sr.reg)}, Target = fetch_reg(R, Aft#sr.reg), - I = case Op of - assoc -> put_map_assoc; - exact -> put_map_exact - end, - {Sis++[Line]++[{I,Fail,SrcReg,Target,Live,{list,List}}],Aft,St}; + {Is,St1} = set_cg_map(Line, Op, SrcReg, Target, Live, List, St0), + {Sis++Is,Aft,St1}; + +%% Everything else. set_cg([{var,R}], Con, Le, Vdb, Bef, St) -> %% Find a place for the return register first. Int = Bef#sr{reg=put_reg(R, Bef#sr.reg)}, @@ -1616,6 +1608,34 @@ set_cg([{var,R}], Con, Le, Vdb, Bef, St) -> end, {Ais,clear_dead(Int, Le#l.i, Vdb),St}. + +set_cg_map(Line, Op0, SrcReg, Target, Live, List, St0) -> + Bfail = St0#cg.bfail, + Fail = {f,St0#cg.bfail}, + Op = case Op0 of + assoc -> put_map_assoc; + exact -> put_map_exact + end, + {OkLbl,St1} = new_label(St0), + {BadLbl,St2} = new_label(St1), + Is = if + Bfail =:= 0 orelse Op =:= put_map_assoc -> + [Line,{Op,{f,0},SrcReg,Target,Live,{list,List}}]; + true -> + %% Ensure that Target is always set, even if + %% the map update operation fails. That is necessary + %% because Target may be included in a test_heap + %% instruction. + [Line, + {Op,{f,BadLbl},SrcReg,Target,Live,{list,List}}, + {jump,{f,OkLbl}}, + {label,BadLbl}, + {move,{atom,ok},Target}, + {jump,Fail}, + {label,OkLbl}] + end, + {Is,St2}. + %%% %%% Code generation for constructing binaries. %%% -- cgit v1.2.3 From a04289a5f30f39b62c7d245f272f486cfd70e6a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 2 Jun 2016 06:40:09 +0200 Subject: Avoid the dreaded "no_file" in warnings Add more filename/line number annotations while translating to Core Erlang in v3_core, and ensure that sys_core_fold retains existing annotations. The goal is to avoid that sys_core_fold generate warnings with "no_file" instead of a filename. --- lib/compiler/src/sys_core_fold.erl | 14 ++++++-------- lib/compiler/src/v3_core.erl | 11 ++++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) (limited to 'lib/compiler/src') diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl index dbc27db377..e0de50f3ae 100644 --- a/lib/compiler/src/sys_core_fold.erl +++ b/lib/compiler/src/sys_core_fold.erl @@ -786,7 +786,7 @@ fold_lit_args(Call, Module, Name, Args0) -> Val -> case cerl:is_literal_term(Val) of true -> - cerl:abstract(Val); + cerl:ann_abstract(cerl:get_ann(Call), Val); false -> %% Successful evaluation, but it was not possible %% to express the computed value as a literal. @@ -2176,24 +2176,22 @@ opt_not_in_let_1(V, Call, Body) -> #c_call{module=#c_literal{val=erlang}, name=#c_literal{val='not'}, args=[#c_var{name=V}]} -> - opt_not_in_let_2(Body); + opt_not_in_let_2(Body, Call); _ -> no end. -opt_not_in_let_2(#c_case{clauses=Cs0}=Case) -> +opt_not_in_let_2(#c_case{clauses=Cs0}=Case, NotCall) -> Vars = make_vars([], 1), - Body = #c_call{module=#c_literal{val=erlang}, - name=#c_literal{val='not'}, - args=Vars}, + Body = NotCall#c_call{args=Vars}, Cs = [begin Let = #c_let{vars=Vars,arg=B,body=Body}, C#c_clause{body=opt_not_in_let(Let)} end || #c_clause{body=B}=C <- Cs0], {yes,Case#c_case{clauses=Cs}}; -opt_not_in_let_2(#c_call{}=Call0) -> +opt_not_in_let_2(#c_call{}=Call0, _NotCall) -> invert_call(Call0); -opt_not_in_let_2(_) -> no. +opt_not_in_let_2(_, _) -> no. invert_call(#c_call{module=#c_literal{val=erlang}, name=#c_literal{val=Name0}, diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl index a3b0236134..d71411de80 100644 --- a/lib/compiler/src/v3_core.erl +++ b/lib/compiler/src/v3_core.erl @@ -868,12 +868,16 @@ try_exception(Ecs0, St0) -> {Evs,St1} = new_vars(3, St0), % Tag, Value, Info {Ecs1,Ceps,St2} = clauses(Ecs0, St1), [_,Value,Info] = Evs, - Ec = #iclause{anno=#a{anno=[compiler_generated]}, + LA = case Ecs1 of + [] -> []; + [C|_] -> get_lineno_anno(C) + end, + Ec = #iclause{anno=#a{anno=[compiler_generated|LA]}, pats=[c_tuple(Evs)],guard=[#c_literal{val=true}], body=[#iprimop{anno=#a{}, %Must have an #a{} name=#c_literal{val=raise}, args=[Info,Value]}]}, - Hs = [#icase{anno=#a{},args=[c_tuple(Evs)],clauses=Ecs1,fc=Ec}], + Hs = [#icase{anno=#a{anno=LA},args=[c_tuple(Evs)],clauses=Ecs1,fc=Ec}], {Evs,Ceps++Hs,St2}. try_after(As, St0) -> @@ -2098,7 +2102,8 @@ upattern(#c_var{name=V}=Var, Ks, St0) -> true -> {N,St1} = new_var_name(St0), New = #c_var{name=N}, - Test = #icall{anno=#a{us=add_element(N, [V])}, + LA = get_lineno_anno(Var), + Test = #icall{anno=#a{anno=LA,us=add_element(N, [V])}, module=#c_literal{val=erlang}, name=#c_literal{val='=:='}, args=[New,Var]}, -- cgit v1.2.3