Skip to content

Commit

Permalink
Merge branch 'bg/compiler-bopt-bug' into ccase/r13b04_dev
Browse files Browse the repository at this point in the history
* bg/compiler-bopt-bug:
  beam_bool: Fix generation of code that does not validate
  Fix crash in beam_bool

OTP-8338  Using complex boolean expressions in ifs could cause the compiler
          to either crash or teminate with an internal error. (Thanks to
          Simon Cornish.)
  • Loading branch information
Erlang/OTP committed Dec 14, 2009
2 parents 7662393 + 88efa63 commit 47b882a
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 14 deletions.
27 changes: 17 additions & 10 deletions lib/compiler/src/beam_bool.erl
Expand Up @@ -123,6 +123,12 @@ bopt_block(Reg, Fail, OldIs, [{block,Bl0}|Acc0], St0) ->
throw:mixed ->
failed;

%% There was a reference to a boolean expression
%% from inside a protected block (try/catch), to
%% a boolean expression outside.
throw:protected_barrier ->
failed;

%% The 'xor' operator was used. We currently don't
%% find it worthwile to translate 'xor' operators
%% (the code would be clumsy).
Expand Down Expand Up @@ -167,7 +173,7 @@ bopt_block(Reg, Fail, OldIs, [{block,Bl0}|Acc0], St0) ->
%% whether the optimized code is guaranteed to work in the same
%% way as the original code.
%%
%% Throws an exception if the optmization is not safe.
%% Throw an exception if the optimization is not safe.
%%
ensure_opt_safe(Bl, NewCode, OldIs, Fail, PreceedingCode, St) ->
%% Here are the conditions that must be true for the
Expand All @@ -184,10 +190,10 @@ ensure_opt_safe(Bl, NewCode, OldIs, Fail, PreceedingCode, St) ->
%% by the code that follows.
%%
%% 3. Any register that is assigned a value in the optimized
%% code must be UNUSED or KILLED in the following code.
%% (Possible future improvement: Registers that are known
%% to be assigned the SAME value in the original and optimized
%% code don't need to be unused in the following code.)
%% code must be UNUSED or KILLED in the following code
%% (because the register might be assigned the wrong value,
%% and even if the value is right it might no longer be
%% assigned on *all* paths leading to its use).

InitInPreceeding = initialized_regs(PreceedingCode),

Expand Down Expand Up @@ -304,6 +310,8 @@ dst_regs([{set,[D],_,{bif,_,{f,_}}}|Is], Acc) ->
dst_regs(Is, [D|Acc]);
dst_regs([{set,[D],_,{alloc,_,{gc_bif,_,{f,_}}}}|Is], Acc) ->
dst_regs(Is, [D|Acc]);
dst_regs([{set,[D],_,move}|Is], Acc) ->
dst_regs(Is, [D|Acc]);
dst_regs([_|Is], Acc) ->
dst_regs(Is, Acc);
dst_regs([], Acc) -> ordsets:from_list(Acc).
Expand Down Expand Up @@ -414,11 +422,10 @@ bopt_good_args([A|As], Regs) ->
bopt_good_args([], _) -> ok.

bopt_good_arg({Tag,_}=X, Regs) when Tag =:= x; Tag =:= tmp ->
case gb_trees:get(X, Regs) of
any -> ok;
_Other ->
%%io:format("not any: ~p: ~p\n", [X,_Other]),
throw(mixed)
case gb_trees:lookup(X, Regs) of
{value,any} -> ok;
{value,_} -> throw(mixed);
none -> throw(protected_barrier)
end;
bopt_good_arg(_, _) -> ok.

Expand Down
66 changes: 62 additions & 4 deletions lib/compiler/test/andor_SUITE.erl
Expand Up @@ -20,13 +20,14 @@

-export([all/1,
t_case/1,t_and_or/1,t_andalso/1,t_orelse/1,inside/1,overlap/1,
combined/1,in_case/1]).
combined/1,in_case/1,before_and_inside_if/1]).

-include("test_server.hrl").

all(suite) ->
test_lib:recompile(?MODULE),
[t_case,t_and_or,t_andalso,t_orelse,inside,overlap,combined,in_case].
[t_case,t_and_or,t_andalso,t_orelse,inside,overlap,combined,in_case,
before_and_inside_if].

t_case(Config) when is_list(Config) ->
%% We test boolean cases almost but not quite like cases
Expand Down Expand Up @@ -380,6 +381,65 @@ in_case_1_guard(LenUp, LenDw, LenN, Rotation, Count) ->
false -> loop
end.

before_and_inside_if(Config) when is_list(Config) ->
?line no = before_and_inside_if([a], [b], delete),
?line no = before_and_inside_if([a], [b], x),
?line no = before_and_inside_if([a], [], delete),
?line no = before_and_inside_if([a], [], x),
?line no = before_and_inside_if([], [], delete),
?line yes = before_and_inside_if([], [], x),
?line yes = before_and_inside_if([], [b], delete),
?line yes = before_and_inside_if([], [b], x),

?line {ch1,ch2} = before_and_inside_if_2([a], [b], blah),
?line {ch1,ch2} = before_and_inside_if_2([a], [b], xx),
?line {ch1,ch2} = before_and_inside_if_2([a], [], blah),
?line {ch1,ch2} = before_and_inside_if_2([a], [], xx),
?line {no,no} = before_and_inside_if_2([], [b], blah),
?line {no,no} = before_and_inside_if_2([], [b], xx),
?line {ch1,no} = before_and_inside_if_2([], [], blah),
?line {no,ch2} = before_and_inside_if_2([], [], xx),
ok.

%% Thanks to Simon Cornish and Kostis Sagonas.
%% Used to crash beam_bool.
before_and_inside_if(XDo1, XDo2, Do3) ->
Do1 = (XDo1 =/= []),
Do2 = (XDo2 =/= []),
if
%% This expression occurs in a try/catch (protected)
%% block, which cannot refer to variables outside of
%% the block that are boolean expressions.
Do1 =:= true;
Do1 =:= false, Do2 =:= false, Do3 =:= delete ->
no;
true ->
yes
end.

%% Thanks to Simon Cornish.
%% Used to generate code that would not set {y,0} on
%% all paths before its use (and therefore fail
%% validation by the beam_validator).
before_and_inside_if_2(XDo1, XDo2, Do3) ->
Do1 = (XDo1 =/= []),
Do2 = (XDo2 =/= []),
CH1 = if Do1 == true;
Do1 == false,Do2==false,Do3 == blah ->
ch1;
true ->
no
end,
CH2 = if Do1 == true;
Do1 == false,Do2==false,Do3 == xx ->
ch2;
true ->
no
end,
{CH1,CH2}.

%% Utilities.

check(V1, V0) ->
if V1 /= V0 ->
io:fwrite("error: ~w.\n", [V1]),
Expand All @@ -393,5 +453,3 @@ echo(X) ->
X.

id(I) -> I.


0 comments on commit 47b882a

Please sign in to comment.