From 53f85bfa09fa0aea9718c207d4245cb975eb6b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 31 Jan 2019 13:03:28 +0100 Subject: Fix internal consistency failure caused by beam_except Fix a bug where the number of live registers in a `bs_get_tail` instruction was too low. Consider this example: -export([bs_get_tail/2]). bs_get_tail(Bin, Config) -> bs_get_tail_1(Bin, 0, 0, Config). bs_get_tail_1(<<_:32, Rest/binary>>, Z1, Z2, F1) -> {Rest,Z1,Z2,F1}. `beam_validator` would emit the following diagnostics: t: function bs_get_tail_1/4+2: Internal consistency check failed - please report this bug. Instruction: {func_info,{atom,t},{atom,bs_get_tail_1},4} Error: {uninitialized_reg,{x,3}}: Here is the part of the code that generates the `function_clause` exception before the optimization: {test_heap,6,4}. {put_list,{x,3},nil,{x,2}}. {put_list,{integer,0},{x,2},{x,2}}. {put_list,{integer,0},{x,2},{x,2}}. {bs_set_position,{x,1},{x,0}}. {bs_get_tail,{x,1},{x,0},3}. %3 live registers. {test_heap,2,3}. {put_list,{x,0},{x,2},{x,1}}. {move,{atom,function_clause},{x,0}}. {line,[{location,"t.erl",8}]}. {call_ext_only,2,{extfunc,erlang,error,2}}. The `bs_get_tail` instruction expects that 3 registers will be live at this point. `beam_except` rewrites the code like this: {bs_set_position,{x,1},{x,0}}. {bs_get_tail,{x,1},{x,0},3}. %Still 3. Too low. {move,{integer,0},{x,1}}. {move,{integer,0},{x,2}}. {jump,{f,3}}. Now the number of live registers in `bs_get_tail` is too low, because the `{x,3}` register will become undefined. This commit adds code to update the number of live registers in the `bs_get_tail` instruction, producing this code: {bs_set_position,{x,1},{x,0}}. {bs_get_tail,{x,1},{x,0},4}. %Adjusted to 4. {move,{integer,0},{x,1}}. {move,{integer,0},{x,2}}. {jump,{f,3}}. --- lib/compiler/src/beam_except.erl | 23 +++++++++++++++++++++-- lib/compiler/test/beam_except_SUITE.erl | 16 +++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) (limited to 'lib/compiler') diff --git a/lib/compiler/src/beam_except.erl b/lib/compiler/src/beam_except.erl index 254a716b81..09925b2872 100644 --- a/lib/compiler/src/beam_except.erl +++ b/lib/compiler/src/beam_except.erl @@ -167,8 +167,10 @@ dig_out_fc_1([{block,Bl}|Is], Regs0, Acc) -> dig_out_fc_1(Is, Regs, Acc); dig_out_fc_1([{bs_set_position,_,_}=I|Is], Regs, Acc) -> dig_out_fc_1(Is, Regs, [I|Acc]); -dig_out_fc_1([{bs_get_tail,_,_,Live}=I|Is], Regs0, Acc) -> - Regs = prune_xregs(Live, Regs0), +dig_out_fc_1([{bs_get_tail,Src,Dst,Live0}|Is], Regs0, Acc) -> + Regs = prune_xregs(Live0, Regs0), + Live = dig_out_stack_live(Regs, Live0), + I = {bs_get_tail,Src,Dst,Live}, dig_out_fc_1(Is, Regs, [I|Acc]); dig_out_fc_1([_|_], _Regs, _Acc) -> {#{},[]}; @@ -189,6 +191,23 @@ dig_out_fc_block([{set,_,_,_}|_], _Regs) -> #{}; dig_out_fc_block([], Regs) -> Regs. +dig_out_stack_live(Regs, Default) -> + Reg = {x,2}, + case Regs of + #{Reg:=List} -> + dig_out_stack_live_1(List, Default); + #{} -> + Default + end. + +dig_out_stack_live_1({cons,{arg,N},T}, Live) -> + dig_out_stack_live_1(T, max(N + 1, Live)); +dig_out_stack_live_1({cons,_,T}, Live) -> + dig_out_stack_live_1(T, Live); +dig_out_stack_live_1(nil, Live) -> + Live; +dig_out_stack_live_1(_, Live) -> Live. + prune_xregs(Live, Regs) -> maps:filter(fun({x,X}, _) -> X < Live end, Regs). diff --git a/lib/compiler/test/beam_except_SUITE.erl b/lib/compiler/test/beam_except_SUITE.erl index 49acad2bc5..9380fe06c8 100644 --- a/lib/compiler/test/beam_except_SUITE.erl +++ b/lib/compiler/test/beam_except_SUITE.erl @@ -21,7 +21,7 @@ -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, init_per_group/2,end_per_group/2, - multiple_allocs/1,coverage/1]). + multiple_allocs/1,bs_get_tail/1,coverage/1]). suite() -> [{ct_hooks,[ts_install_cth]}]. @@ -31,6 +31,7 @@ all() -> groups() -> [{p,[parallel], [multiple_allocs, + bs_get_tail, coverage]}]. init_per_suite(Config) -> @@ -63,6 +64,17 @@ place(lee) -> conditions() -> (talking = going) = storage + [large = wanted]. +bs_get_tail(Config) -> + {<<"abc">>,0,0,Config} = bs_get_tail_1(id(<<0:32, "abc">>), 0, 0, Config), + {'EXIT', + {function_clause, + [{?MODULE,bs_get_tail_1,[<<>>,0,0,Config],_}|_]}} = + (catch bs_get_tail_1(id(<<>>), 0, 0, Config)), + ok. + +bs_get_tail_1(<<_:32, Rest/binary>>, Z1, Z2, F1) -> + {Rest,Z1,Z2,F1}. + coverage(_) -> File = {file,"fake.erl"}, ok = fc(a), @@ -99,6 +111,8 @@ coverage(_) -> fake_function_clause(A) -> error(function_clause, [A,42.0]). +id(I) -> I. + -file("fake.erl", 1). fc(a) -> %Line 2 ok; %Line 3 -- cgit v1.2.3