aboutsummaryrefslogtreecommitdiffstats
path: root/lib/compiler/test/bs_match_SUITE.erl
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2016-10-06 06:01:11 +0200
committerBjörn Gustavsson <[email protected]>2016-10-06 07:10:37 +0200
commit67808ef46f8f429652ddebb81b0b5c3c603f8655 (patch)
treec156df806f0b7de28740af748283476ff3aa02b3 /lib/compiler/test/bs_match_SUITE.erl
parent05be21eb0ad09fcdd63d955ed1d5b50ed34af925 (diff)
downloadotp-67808ef46f8f429652ddebb81b0b5c3c603f8655.tar.gz
otp-67808ef46f8f429652ddebb81b0b5c3c603f8655.tar.bz2
otp-67808ef46f8f429652ddebb81b0b5c3c603f8655.zip
beam_bsm: Eliminate unsafe optimization
The following code causes a compiler failure: first_after(Data, Offset) -> case byte_size(Data) > Offset of false -> {First, Rest} = {ok, ok}, ok; true -> <<_:Offset/binary, Rest/binary>> = Data, %% 'Rest' saved in y(0) before the call. {First, _} = match_first(Data, Rest), %% When beam_bsm sees the code, the following line %% which uses y(0) has been optimized away. {First, Rest} = {First, Rest}, First end. match_first(_, <<First:1/binary, Rest/binary>>) -> {First, Rest}. Here is the error message from beam_validator: t: function first_after/2+15: Internal consistency check failed - please report this bug. Instruction: {call,2,{f,7}} Error: {multiple_match_contexts,[{x,1},0]}: Basically, what happens is that at time of code generation, the variable 'Rest' is needed after the call to match_first/2 and is therefore saved in y(0). When beam_bsm (a late optimization pass) sees the code, the use of y(0) following the call to match_first/2 has been optimized away. beam_bsm therefore assumes that the delayed sub-binary creation is safe. (Actually, it is safe, but beam_validator does not realize it.) The bug was caused by two separate commits: e199e2471a reduced the number of special cases to handle in BEAM optimization passed by breaking apart the tail-recursive call instructions (call_only and call_last) into separate instructions. Unfortunately, the special handling for tail calls was lost, which resulted in worse code (i.e. the delayed sub-binary creation optimization could not be applied). e1aa422290 tried to compensate, but did so in a way that was not always safe. Teaching beam_validator that this kind of code is safe would be expensive. Instead, we will undo the damage caused by the two commits. Re-introduce the special handling of tail-recursive calls in beam_bsm that was lost in the first commit. (Effectively) revert the change in the second commit. ERL-268
Diffstat (limited to 'lib/compiler/test/bs_match_SUITE.erl')
-rw-r--r--lib/compiler/test/bs_match_SUITE.erl26
1 files changed, 25 insertions, 1 deletions
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
index 224abf6c29..a9bee888d9 100644
--- a/lib/compiler/test/bs_match_SUITE.erl
+++ b/lib/compiler/test/bs_match_SUITE.erl
@@ -38,7 +38,8 @@
no_partition/1,calling_a_binary/1,binary_in_map/1,
match_string_opt/1,select_on_integer/1,
map_and_binary/1,unsafe_branch_caching/1,
- bad_literals/1,good_literals/1,constant_propagation/1]).
+ bad_literals/1,good_literals/1,constant_propagation/1
+ ]).
-export([coverage_id/1,coverage_external_ignore/2]).
@@ -768,6 +769,11 @@ multiple_uses(Config) when is_list(Config) ->
{344,62879,345,<<245,159,1,89>>} = multiple_uses_1(<<1,88,245,159,1,89>>),
true = multiple_uses_2(<<0,0,197,18>>),
<<42,43>> = multiple_uses_3(<<0,0,42,43>>, fun id/1),
+
+ ok = first_after(<<>>, 42),
+ <<1>> = first_after(<<1,2,3>>, 0),
+ <<2>> = first_after(<<1,2,3>>, 1),
+
ok.
multiple_uses_1(<<X:16,Tail/binary>>) ->
@@ -789,6 +795,24 @@ multiple_uses_match(<<Y:16,Z:16>>) ->
multiple_uses_cmp(<<Y:16>>, <<Y:16>>) -> true;
multiple_uses_cmp(<<_:16>>, <<_:16>>) -> false.
+first_after(Data, Offset) ->
+ case byte_size(Data) > Offset of
+ false ->
+ {First, Rest} = {ok, ok},
+ ok;
+ true ->
+ <<_:Offset/binary, Rest/binary>> = Data,
+ %% 'Rest' saved in y(0) before the call.
+ {First, _} = match_first(Data, Rest),
+ %% When beam_bsm sees the code, the following line
+ %% which uses y(0) has been optimized away.
+ {First, Rest} = {First, Rest},
+ First
+ end.
+
+match_first(_, <<First:1/binary, Rest/binary>>) ->
+ {First, Rest}.
+
zero_label(Config) when is_list(Config) ->
<<"nosemouth">> = read_pols(<<"FACE","nose","mouth">>),
<<"CE">> = read_pols(<<"noFACE">>),