From af198c489a7ab431fd1e2b52d16e8e13525915f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= 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/test/andor_SUITE.erl | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) (limited to 'lib/compiler/test') 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 From 88efa63b733b627934fb7eec6236c71d7acfe082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 11 Dec 2009 22:25:51 +0100 Subject: beam_bool: Fix generation of code that does not validate The following code (by Simon Cornish) bad(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}. is optimized by beam_bool even though the optimization is not safe. The trouble is that an assignment to {y,0} no longer occurs on all paths leading to its use. The bug is in dst_regs/2 which is supposed to return a set of all registers assigned in a code block, but it ignores registers assigned in 'move' instructions. Fix the bug by taking 'move' instructions into account. This change is safe since it can only cause more registers to be added to the MustBeKilled and MustBeUnused sets in ensure_opt_safe/6, which means that it can only cause the optimization to be turned off for code that used to be optimized. --- lib/compiler/test/andor_SUITE.erl | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'lib/compiler/test') diff --git a/lib/compiler/test/andor_SUITE.erl b/lib/compiler/test/andor_SUITE.erl index 6e3ac4d4f4..a460d54239 100644 --- a/lib/compiler/test/andor_SUITE.erl +++ b/lib/compiler/test/andor_SUITE.erl @@ -390,6 +390,15 @@ before_and_inside_if(Config) when is_list(Config) -> ?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. @@ -408,6 +417,27 @@ before_and_inside_if(XDo1, XDo2, Do3) -> 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) -> -- cgit v1.2.3