From af198c489a7ab431fd1e2b52d16e8e13525915f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Thu, 10 Dec 2009 21:18:39 +0100
Subject: Fix crash in beam_bool

The following code crashes beam_bool:

bad(XDo1, XDo2, Do3) ->
    Do1 = (XDo1 =/= []),
    Do2 = (XDo2 =/= []),
    if Do1 =:= true;
       Do1 =:= false, Do2 =:= false, Do3 =:= delete ->
	    no
    end.

(Reported by Simon Cornish; minimized by Kostis Sagonas.)

For the moment fix the bug in the simplest and safest way possible
(basically, instead of crashing just don't do the optimization).

In a future major release (e.g. R14), the following improvements
could be considered:

* In beam_bool, it should be possible to move the Do1 and Do2
  expressions to the pre-block and still optimize the expression
  in the 'if' statement.

* In sys_core_fold, it should be possible to eliminate the
  try/catch around the guard expression in the 'if', because
  none of the guard tests can actually fail.
---
 lib/compiler/src/beam_bool.erl    | 15 ++++++++++-----
 lib/compiler/test/andor_SUITE.erl | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 9 deletions(-)

(limited to 'lib/compiler')

diff --git a/lib/compiler/src/beam_bool.erl b/lib/compiler/src/beam_bool.erl
index d8c201a194..ffe5cdb501 100644
--- a/lib/compiler/src/beam_bool.erl
+++ b/lib/compiler/src/beam_bool.erl
@@ -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).
@@ -414,11 +420,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.
 
diff --git a/lib/compiler/test/andor_SUITE.erl b/lib/compiler/test/andor_SUITE.erl
index 34609a49f2..6e3ac4d4f4 100644
--- a/lib/compiler/test/andor_SUITE.erl
+++ b/lib/compiler/test/andor_SUITE.erl
@@ -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
@@ -380,6 +381,35 @@ 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),
+    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.
+
+%% Utilities.
+
 check(V1, V0) ->
     if V1 /= V0 ->
 	    io:fwrite("error: ~w.\n", [V1]),
@@ -393,5 +423,3 @@ echo(X) ->
     X.
 
 id(I) -> I.
-
-    
-- 
cgit v1.2.3