aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2013-01-18 14:34:08 +0100
committerBjörn Gustavsson <[email protected]>2013-01-18 16:51:08 +0100
commit67195b2a1dddf1ec2590e1404b552188893d4473 (patch)
tree6aa43b9eed7666f5740bb9ffdb6c091d1c578294
parent2bd44c7f5462bbfaaf38eed6f708bf01b0c97469 (diff)
downloadotp-67195b2a1dddf1ec2590e1404b552188893d4473.tar.gz
otp-67195b2a1dddf1ec2590e1404b552188893d4473.tar.bz2
otp-67195b2a1dddf1ec2590e1404b552188893d4473.zip
compiler: Eliminate internal consistency failure in binary matching
The following code: check(<<"string">>, a1) -> one; check(_, a2) -> two; check(undefined, a3) -> three. produces an internal consistency failure: check: function check/2+17: Internal consistency check failed - please report this bug. Instruction: {test,is_eq_exact,{f,7},[{x,0},{atom,undefined}]} Error: {match_context,{x,0}}: Actually, in the current implementation of the run-time system, comparing a match context to an atom is safe, so I briefly considered updating the beam_validator to let this code pass through. I abandoned that approach because not all terms would be safe to compare to a match context, and the implementation might change in the future. Therefore, fix this problem by not allowing any matching of non-variables (in the argument position for binary being matched) following binary matching. That solution is simple and safe, and since this kind of code seems to be rare in practice, there is no need to pursue any more compilicated solution. Reported-by: Viktor Sovietov
-rw-r--r--lib/compiler/src/sys_core_fold.erl37
-rw-r--r--lib/compiler/test/bs_match_SUITE.erl48
2 files changed, 73 insertions, 12 deletions
diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl
index f17b0bd130..fbd7452301 100644
--- a/lib/compiler/src/sys_core_fold.erl
+++ b/lib/compiler/src/sys_core_fold.erl
@@ -2672,16 +2672,19 @@ bsm_nonempty([#c_clause{pats=Ps}|Cs], Pos) ->
bsm_nonempty([], _ ) -> false.
%% bsm_ensure_no_partition(Cs, Pos) -> ok (exception if problem)
-%% We must make sure that binary matching is not partitioned between
+%% We must make sure that matching is not partitioned between
%% variables like this:
%% foo(<<...>>) -> ...
-%% foo(Var) when ... -> ...
-%% foo(<<...>>) ->
+%% foo(<Variable>) when ... -> ...
+%% foo(<Any non-variable pattern>) ->
%% If there is such partition, we are not allowed to reuse the binary variable
-%% for the match context. Also, arguments to the left of the argument that
-%% is matched against a binary, are only allowed to be simple variables, not
-%% used in guards. The reason is that we must know that the binary is only
-%% matched in one place.
+%% for the match context.
+%%
+%% Also, arguments to the left of the argument that is matched
+%% against a binary, are only allowed to be simple variables, not
+%% used in guards. The reason is that we must know that the binary is
+%% only matched in one place (i.e. there must be only one bs_start_match2
+%% instruction emitted).
bsm_ensure_no_partition(Cs, Pos) ->
bsm_ensure_no_partition_1(Cs, Pos, before).
@@ -2689,6 +2692,12 @@ bsm_ensure_no_partition(Cs, Pos) ->
%% Loop through each clause.
bsm_ensure_no_partition_1([#c_clause{pats=Ps,guard=G}|Cs], Pos, State0) ->
State = bsm_ensure_no_partition_2(Ps, Pos, G, simple_vars, State0),
+ case State of
+ 'after' ->
+ bsm_ensure_no_partition_after(Cs, Pos);
+ _ ->
+ ok
+ end,
bsm_ensure_no_partition_1(Cs, Pos, State);
bsm_ensure_no_partition_1([], _, _) -> ok.
@@ -2698,8 +2707,7 @@ bsm_ensure_no_partition_2([#c_binary{}=Where|_], 1, _, Vstate, State) ->
before when Vstate =:= simple_vars -> within;
before -> bsm_problem(Where, Vstate);
within when Vstate =:= simple_vars -> within;
- within -> bsm_problem(Where, Vstate);
- 'after' -> bsm_problem(Where, bin_partition)
+ within -> bsm_problem(Where, Vstate)
end;
bsm_ensure_no_partition_2([#c_alias{}=Alias|_], 1, N, Vstate, State) ->
%% Retrieve the real pattern that the alias refers to and check that.
@@ -2748,6 +2756,15 @@ bsm_ensure_no_partition_2([#c_var{name=V}|Ps], N, G, Vstate, S) ->
bsm_ensure_no_partition_2([_|Ps], N, G, _, S) ->
bsm_ensure_no_partition_2(Ps, N-1, G, bin_argument_order, S).
+bsm_ensure_no_partition_after([#c_clause{pats=Ps}|Cs], Pos) ->
+ case nth(Pos, Ps) of
+ #c_var{} ->
+ bsm_ensure_no_partition_after(Cs, Pos);
+ P ->
+ bsm_problem(P, bin_partition)
+ end;
+bsm_ensure_no_partition_after([], _) -> ok.
+
bsm_could_match_binary(#c_alias{pat=P}) -> bsm_could_match_binary(P);
bsm_could_match_binary(#c_cons{}) -> false;
bsm_could_match_binary(#c_tuple{}) -> false;
@@ -2872,7 +2889,7 @@ format_error(useless_building) ->
format_error(bin_opt_alias) ->
"INFO: the '=' operator will prevent delayed sub binary optimization";
format_error(bin_partition) ->
- "INFO: non-consecutive clauses that match binaries "
+ "INFO: matching non-variables after a previous clause matching a variable "
"will prevent delayed sub binary optimization";
format_error(bin_left_var_used_in_guard) ->
"INFO: a variable to the left of the binary pattern is used in a guard; "
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
index d63d2235d7..e8a92c509e 100644
--- a/lib/compiler/test/bs_match_SUITE.erl
+++ b/lib/compiler/test/bs_match_SUITE.erl
@@ -33,7 +33,8 @@
matching_meets_construction/1,simon/1,matching_and_andalso/1,
otp_7188/1,otp_7233/1,otp_7240/1,otp_7498/1,
match_string/1,zero_width/1,bad_size/1,haystack/1,
- cover_beam_bool/1,matched_out_size/1,follow_fail_branch/1]).
+ cover_beam_bool/1,matched_out_size/1,follow_fail_branch/1,
+ no_partition/1]).
-export([coverage_id/1,coverage_external_ignore/2]).
@@ -57,7 +58,8 @@ groups() ->
matching_meets_construction,simon,
matching_and_andalso,otp_7188,otp_7233,otp_7240,
otp_7498,match_string,zero_width,bad_size,haystack,
- cover_beam_bool,matched_out_size,follow_fail_branch]}].
+ cover_beam_bool,matched_out_size,follow_fail_branch,
+ no_partition]}].
init_per_suite(Config) ->
@@ -1133,6 +1135,48 @@ ffb_2(<<_,T/bitstring>>, List, A) ->
[_|_] -> bit_size(T)
end.
+no_partition(_) ->
+ one = no_partition_1(<<"string">>, a1),
+ {two,<<"string">>} = no_partition_1(<<"string">>, a2),
+ {two,<<>>} = no_partition_1(<<>>, a2),
+ {two,a} = no_partition_1(a, a2),
+ three = no_partition_1(undefined, a3),
+ {four,a,[]} = no_partition_1([a], a4),
+ {five,a,b} = no_partition_1({a,b}, a5),
+
+ one = no_partition_2(<<"string">>, a1),
+ two = no_partition_2(<<"string">>, a2),
+ two = no_partition_2(<<>>, a2),
+ two = no_partition_2(a, a2),
+ three = no_partition_2(undefined, a3),
+ four = no_partition_2(42, a4),
+ five = no_partition_2([], a5),
+ six = no_partition_2(42.0, a6),
+ ok.
+
+no_partition_1(<<"string">>, a1) ->
+ one;
+no_partition_1(V, a2) ->
+ {two,V};
+no_partition_1(undefined, a3) ->
+ three;
+no_partition_1([H|T], a4) ->
+ {four,H,T};
+no_partition_1({A,B}, a5) ->
+ {five,A,B}.
+
+no_partition_2(<<"string">>, a1) ->
+ one;
+no_partition_2(_, a2) ->
+ two;
+no_partition_2(undefined, a3) ->
+ three;
+no_partition_2(42, a4) ->
+ four;
+no_partition_2([], a5) ->
+ five;
+no_partition_2(42.0, a6) ->
+ six.
check(F, R) ->
R = F().