beam_bool: Correct live calculation for GC BIFs

When optimizing boolean expressions, it is not always possible
to find a number of live registers for a GC BIF that both preserves
all source registers that will be tested and at the same time
does not include registers that are not initialized.

As currently implemented, we have incomplete information about
the register calculated from the free variables. Some registers
are marked as "reserved". Reserved registers means that we don't
know anything about them; they may or may not be initialized.

As a conservative correction (suitable for a maintenance release), we
will abort the optimization if we find any reserved registers when
calculating the number of live registers. We will not attempt to
improve the information about the registers in this commit.

By examining the coverage when running the existing compiler test
suite we find that the optimization is aborted 15 times (before
adding any new test cases). To put that in perspective, the
optimization is successfully applied 4927 times, and aborted for
other reasons 547 times.

Reported-by: Ulf Norell
Reported-by: Anthony Ramine

2 files changed, 37 insertions, 4 deletions

diff --git a/lib/compiler/src/beam_bool.erl b/lib/compiler/src/beam_bool.erl
index dc4d7927e9..a452d30b61 100644
--- a/lib/compiler/src/beam_bool.erl
+++ b/lib/compiler/src/beam_bool.erl
@@ -163,7 +163,16 @@ bopt_block(Reg, Fail, OldIs, [{block,Bl0}|Acc0], St0) ->
 		%% in guards, so it must have been another
 		%% Core Erlang translator.)
 		throw:protected_violation ->
+		    failed;
+
+		%% Failed to work out the live registers for a GC
+		%% BIF. For example, if the number of live registers
+		%% needed to be 4 because {x,3} was a source register,
+		%% but {x,2} was not known to be initialized, this
+		%% exception would be thrown.
+		throw:gc_bif_alloc_failure ->
 		    failed
+
 	    end
     end.
 
@@ -665,10 +674,16 @@ put_reg_1(V, [], I) -> [{I,V}].
 fetch_reg(V, [{I,V}|_]) -> {x,I};
 fetch_reg(V, [_|SRs]) -> fetch_reg(V, SRs).
 
-live_regs(Regs) ->
-    foldl(fun ({I,_}, _) ->
-		  I
-	  end, -1, Regs)+1.
+live_regs([{_,reserved}|_]) ->
+    %% We are not sure that this register is initialized, so we must
+    %% abort the optimization.
+    throw(gc_bif_alloc_failure);
+live_regs([{I,_}]) ->
+    I+1;
+live_regs([{_,_}|Regs]) ->
+    live_regs(Regs);
+live_regs([]) ->
+    0.
 
     
 %%%
diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl
index eb205d09a7..34bfdeb1e5 100644
--- a/lib/compiler/test/guard_SUITE.erl
+++ b/lib/compiler/test/guard_SUITE.erl
@@ -1556,6 +1556,24 @@ bad_constants(Config) when is_list(Config) ->
 
 bad_guards(Config) when is_list(Config) ->
     if erlang:float(self()); true -> ok end,
+
+    fc(catch bad_guards_1(1, [])),
+    fc(catch bad_guards_1(1, [2])),
+    fc(catch bad_guards_1(atom, [2])),
+
+    fc(catch bad_guards_2(#{a=>0,b=>0}, [])),
+    fc(catch bad_guards_2(#{a=>0,b=>0}, [x])),
+    fc(catch bad_guards_2(not_a_map, [x])),
+    fc(catch bad_guards_2(42, [x])),
+    ok.
+
+%% beam_bool used to produce GC BIF instructions whose
+%% Live operands included uninitialized registers.
+
+bad_guards_1(X, [_]) when {{X}}, -X ->
+    ok.
+
+bad_guards_2(M, [_]) when M#{a := 0, b => 0}, map_size(M) ->
     ok.
 
 %% Call this function to turn off constant propagation.