Eliminate crash because of unsafe delaying of sub-binary creation

The following code would fail to compile:

decode(<<Code/integer, Bin/binary>>) ->
    <<C1/integer, B1/binary>> = Bin,
    case C1 of
	X when X =:= 1 orelse X =:= 2 ->
	    Bin2 = <<>>;
	_ ->
	    Bin2 = B1
    end,
    case Code of
	1 -> decode(Bin2);
	_ -> Bin2
    end.

The error message would be:

t: function decode/1+28:
  Internal consistency check failed - please report this bug.
  Instruction: return
  Error:       {match_context,{x,0}}:

The beam_bsm pass would delay the creation of a sub-binary when it was
unsafe to do so. The culprit was the btb_follow_branch/3 function that
for performance reasons cached labels that had already been checked.
The problem was the safety of a label also depends on the contents
of the registers. Therefore, the key for caching needs to be both
the label and the register contents.

Reported-by: José Valim

2 files changed, 33 insertions, 4 deletions

diff --git a/lib/compiler/src/beam_bsm.erl b/lib/compiler/src/beam_bsm.erl
index 4f76350269..62356928ae 100644
--- a/lib/compiler/src/beam_bsm.erl
+++ b/lib/compiler/src/beam_bsm.erl
@@ -421,7 +421,8 @@ btb_follow_branches([], _, D) -> D.
 
 btb_follow_branch(0, _Regs, D) -> D;
 btb_follow_branch(Lbl, Regs, #btb{ok_br=Br0,index=Li}=D) ->
-    case gb_sets:is_member(Lbl, Br0) of
+    Key = {Lbl,Regs},
+    case gb_sets:is_member(Key, Br0) of
 	true ->
 	    %% We have already followed this branch and it was OK.
 	    D;
@@ -432,7 +433,7 @@ btb_follow_branch(Lbl, Regs, #btb{ok_br=Br0,index=Li}=D) ->
 		btb_reaches_match_1(Is, Regs, D),
 
 	    %% Since we got back, this branch is OK.
-	    D#btb{ok_br=gb_sets:insert(Lbl, Br),must_not_save=MustNotSave,
+	    D#btb{ok_br=gb_sets:insert(Key, Br),must_not_save=MustNotSave,
 		  must_save=MustSave}
     end.
 
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
index b4601b0798..7fb0a16540 100644
--- a/lib/compiler/test/bs_match_SUITE.erl
+++ b/lib/compiler/test/bs_match_SUITE.erl
@@ -36,7 +36,8 @@
 	 match_string/1,zero_width/1,bad_size/1,haystack/1,
 	 cover_beam_bool/1,matched_out_size/1,follow_fail_branch/1,
 	 no_partition/1,calling_a_binary/1,binary_in_map/1,
-	 match_string_opt/1,map_and_binary/1]).
+	 match_string_opt/1,map_and_binary/1,
+	 unsafe_branch_caching/1]).
 
 -export([coverage_id/1,coverage_external_ignore/2]).
 
@@ -62,7 +63,8 @@ groups() ->
        otp_7498,match_string,zero_width,bad_size,haystack,
        cover_beam_bool,matched_out_size,follow_fail_branch,
        no_partition,calling_a_binary,binary_in_map,
-       match_string_opt,map_and_binary]}].
+       match_string_opt,map_and_binary,
+       unsafe_branch_caching]}].
 
 
 init_per_suite(Config) ->
@@ -1243,6 +1245,32 @@ do_map_and_binary(#{time := _} = T) ->
 do_map_and_binary(#{hour := Hour, min := Min} = T) ->
     {Hour, Min, T}.
 
+%% Unsafe caching of branch outcomes in beam_bsm would cause the
+%% delayed creation of sub-binaries optimization to be applied even
+%% when it was unsafe.
+
+unsafe_branch_caching(_Config) ->
+    <<>> = do_unsafe_branch_caching(<<42,1>>),
+    <<>> = do_unsafe_branch_caching(<<42,2>>),
+    <<>> = do_unsafe_branch_caching(<<42,3>>),
+    <<17,18>> = do_unsafe_branch_caching(<<42,3,17,18>>),
+    <<>> = do_unsafe_branch_caching(<<1,3,42,2>>),
+
+    ok.
+
+do_unsafe_branch_caching(<<Code/integer, Bin/binary>>) ->
+    <<C1/integer, B1/binary>> = Bin,
+    case C1 of
+	X when X =:= 1 orelse X =:= 2 ->
+	    Bin2 = <<>>;
+	_ ->
+	    Bin2 = B1
+    end,
+    case Code of
+	1 -> do_unsafe_branch_caching(Bin2);
+	_ -> Bin2
+    end.
+
 
 check(F, R) ->
     R = F().