aboutsummaryrefslogtreecommitdiffstats
path: root/lib/compiler
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2018-08-07 12:52:33 +0200
committerBjörn Gustavsson <[email protected]>2018-08-08 13:57:24 +0200
commit2e40d8d1c51ad1c3d3750490ecac6b290233f085 (patch)
tree8807a55b35ad9f3a8bacd03e2f95ece8fedd75c6 /lib/compiler
parentce0ab49ca59dc56cb4774abc1f635b7ff9cf97df (diff)
downloadotp-2e40d8d1c51ad1c3d3750490ecac6b290233f085.tar.gz
otp-2e40d8d1c51ad1c3d3750490ecac6b290233f085.tar.bz2
otp-2e40d8d1c51ad1c3d3750490ecac6b290233f085.zip
Fix bug in binary matching
The compiler generates incorrect code for the following example: decode_binary(_, <<Length, Data/binary>>) -> case {Length, Data} of {0, _} -> %% When converting the match context back to a binary, %% Data will be set to the entire original binary, %% that is, to <<0>> instead of <<>>. {{0, 0, 0}, Data}; {4, <<Y:16/little, M, D, Rest/binary>>} -> {{Y, M, D}, Rest} end. The problem is the delayed sub binary creation optimization, which is not safe to do in this case. This commit introduces a heuristic that will disable the delayed sub binary creation optimization for this example. Unfortunately, the heuristic may turn off the optimization when it would actually be safe. In the OTP codebase, the optimization is turned off in two instances, once in string.erl and once in dets_v9.erl. https://bugs.erlang.org/browse/ERL-689
Diffstat (limited to 'lib/compiler')
-rw-r--r--lib/compiler/src/beam_bsm.erl13
-rw-r--r--lib/compiler/test/bs_match_SUITE.erl33
2 files changed, 43 insertions, 3 deletions
diff --git a/lib/compiler/src/beam_bsm.erl b/lib/compiler/src/beam_bsm.erl
index 1c8e0e9854..abc6e96c85 100644
--- a/lib/compiler/src/beam_bsm.erl
+++ b/lib/compiler/src/beam_bsm.erl
@@ -310,7 +310,18 @@ btb_reaches_match_2([{test,bs_start_match2,{f,F},Live,[Bin,_],Ctx}|Is],
end;
btb_reaches_match_2([{test,_,{f,F},Ss}=I|Is], Regs, D0) ->
btb_ensure_not_used(Ss, I, Regs),
- D = btb_follow_branch(F, Regs, D0),
+ D1 = btb_follow_branch(F, Regs, D0),
+ D = case Is of
+ [{bs_context_to_binary,_}|_] ->
+ %% bs_context_to_binary following a test instruction
+ %% probably needs the current position to be saved as
+ %% the new start position, but we can't be sure.
+ %% Therefore, conservatively disable the optimization
+ %% (instead of forcing a saving of the position).
+ D1#btb{must_save=true,must_not_save=true};
+ _ ->
+ D1
+ end,
btb_reaches_match_1(Is, Regs, D);
btb_reaches_match_2([{test,_,{f,F},_,Ss,_}=I|Is], Regs, D0) ->
btb_ensure_not_used(Ss, I, Regs),
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
index e737839575..7814738449 100644
--- a/lib/compiler/test/bs_match_SUITE.erl
+++ b/lib/compiler/test/bs_match_SUITE.erl
@@ -40,7 +40,7 @@
map_and_binary/1,unsafe_branch_caching/1,
bad_literals/1,good_literals/1,constant_propagation/1,
parse_xml/1,get_payload/1,escape/1,num_slots_different/1,
- beam_bsm/1,guard/1,is_ascii/1,non_opt_eq/1]).
+ beam_bsm/1,guard/1,is_ascii/1,non_opt_eq/1,erl_689/1]).
-export([coverage_id/1,coverage_external_ignore/2]).
@@ -72,7 +72,7 @@ groups() ->
map_and_binary,unsafe_branch_caching,
bad_literals,good_literals,constant_propagation,parse_xml,
get_payload,escape,num_slots_different,
- beam_bsm,guard,is_ascii,non_opt_eq]}].
+ beam_bsm,guard,is_ascii,non_opt_eq,erl_689]}].
init_per_suite(Config) ->
@@ -1688,6 +1688,35 @@ non_opt_eq([_|_], <<_,_/binary>>) ->
non_opt_eq([], <<>>) ->
true.
+%% ERL-689
+
+erl_689(Config) ->
+ {{0, 0, 0}, <<>>} = do_erl_689_1(<<0>>, ?MODULE),
+ {{2018, 8, 7}, <<>>} = do_erl_689_1(<<4,2018:16/little,8,7>>, ?MODULE),
+ {{0, 0, 0}, <<>>} = do_erl_689_2(?MODULE, <<0>>),
+ {{2018, 8, 7}, <<>>} = do_erl_689_2(?MODULE, <<4,2018:16/little,8,7>>),
+ ok.
+
+do_erl_689_1(<<Length, Data/binary>>, _) ->
+ case {Data, Length} of
+ {_, 0} ->
+ %% bs_context_to_binary would incorrectly set Data to the original
+ %% binary (before matching in the function head).
+ {{0, 0, 0}, Data};
+ {<<Y:16/little, M, D, Rest/binary>>, 4} ->
+ {{Y, M, D}, Rest}
+ end.
+
+do_erl_689_2(_, <<Length, Data/binary>>) ->
+ case {Length, Data} of
+ {0, _} ->
+ %% bs_context_to_binary would incorrectly set Data to the original
+ %% binary (before matching in the function head).
+ {{0, 0, 0}, Data};
+ {4, <<Y:16/little, M, D, Rest/binary>>} ->
+ {{Y, M, D}, Rest}
+ end.
+
check(F, R) ->
R = F().