beam_ssa_opt: Fix block tracking in bsm_shortcut

jhogberg · jhogberg · commit a1d8cc1ec330 · 2025-07-16T10:37:07.000+02:00
Success blocks were not adequately tracked, causing `bs_test_tail` instructions to be removed when they shouldn't have been. Fixes GH-10047
diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl
@@ -2002,8 +2002,7 @@ bsm_skip([], _) -> [].
 
 bsm_skip_is([I0|Is], Extracted) ->
     case I0 of
-        #b_set{anno=Anno0,
-               op=bs_match,
+        #b_set{op=bs_match,
                dst=Ctx,
                args=[#b_literal{val=T}=Type,PrevCtx|Args0]}
           when T =/= float, T =/= string, T =/= skip ->
@@ -2014,9 +2013,7 @@ bsm_skip_is([I0|Is], Extracted) ->
                         I0;
                     false ->
                         %% The value is never extracted.
-                        Args = [#b_literal{val=skip},PrevCtx,Type|Args0],
-                        Anno = maps:remove(arg_types, Anno0),
-                        I0#b_set{anno=Anno,args=Args}
+                        I0#b_set{args=[#b_literal{val=skip},PrevCtx,Type|Args0]}
                 end,
             [I|Is];
         #b_set{} ->
@@ -2117,7 +2114,7 @@ ssa_opt_bsm_shortcut({#opt_st{ssa=Linear0}=St, FuncDb}) ->
             {St, FuncDb};
         _ ->
             Linear1 = bsm_shortcut(Linear0, Positions),
-            Linear = bsm_tail(Linear1, #{}),
+            Linear = bsm_tail(Linear1, #{ 0 => any }),
             ssa_opt_live({St#opt_st{ssa=Linear}, FuncDb})
     end.
 
@@ -2200,86 +2197,105 @@ bsm_shortcut([], _PosMap) -> [].
 %%     m1(<<_, Rest/binary>>) -> m1(Rest);
 %%     m1(<<>>) -> ok.
 %%
-%% The second clause of `m1/1` does not need to check for an empty
-%% binary.
-
-bsm_tail([{L,#b_blk{is=Is0,last=Last0}=Blk0}|Bs], Map0) ->
-    {Is,Last,Map} = bsm_tail_is(Is0, Last0, L, Map0, []),
-    Blk = Blk0#b_blk{is=Is,last=Last},
-    [{L,Blk}|bsm_tail(Bs, Map)];
-bsm_tail([], _Map) ->
+%% The second clause of `m1/1` does not need to check for an empty bitstring.
+%%
+%% This is done by keeping track of which blocks are reachable solely because
+%% of `bs_match` instructions that can only fail because the end has been
+%% reached, and then eliminating the related `bs_match` and `bs_test_tail`
+%% instructions in those blocks.
+
+bsm_tail([{L, #b_blk{is=Is0}=Blk0} | Bs], Tags0) when is_map_key(L, Tags0) ->
+    {Blk, Tags} = bsm_tail_is_1(Is0, Blk0, L, Tags0),
+    [{L, Blk} | bsm_tail(Bs, Tags)];
+bsm_tail([_ | Bs], Tags) ->
+    bsm_tail(Bs, Tags);
+bsm_tail([], _Tags) ->
     [].
 
-bsm_tail_is([#b_set{op=bs_start_match,anno=Anno,dst=Dst}=I|Is], Last, L, Map0, Acc) ->
-    case Anno of
-        #{arg_types := #{1 := Type}} ->
-            case beam_types:get_bs_matchable_unit(Type) of
-                error ->
-                    bsm_tail_is(Is, Last, L, Map0, [I|Acc]);
-                Unit when is_integer(Unit) ->
-                    Map = Map0#{Dst => Unit},
-                    bsm_tail_is(Is, Last, L, Map, [I|Acc])
-            end;
-        #{} ->
-            bsm_tail_is(Is, Last, L, Map0, [I|Acc])
-    end;
-bsm_tail_is([#b_set{op=bs_match,dst=Dst,args=Args},
-             #b_set{op={succeeded,guard},dst=SuccDst,args=[Dst]}|_]=Is,
-            #b_br{bool=SuccDst,fail=Fail}=Last,
-            _L, Map0, Acc) ->
-    case bsm_tail_num_matched(Args, Map0) of
-        unknown ->
-            %% Unknown number of bits or the match operation will fail
-            %% to match certain values.
-            Map = Map0#{Fail => unknown},
-            {reverse(Acc, Is),Last,Map};
-        Bits when is_integer(Bits) ->
-            case Map0 of
-                #{Fail := Bits} ->
-                    {reverse(Acc, Is),Last,Map0};
-                #{Fail := _} ->
-                    Map = Map0#{Fail => unknown},
-                    {reverse(Acc, Is),Last,Map};
-                #{} ->
-                    Map = Map0#{Fail => Bits},
-                    {reverse(Acc, Is),Last,Map}
-            end
-    end;
-bsm_tail_is([#b_set{op=bs_test_tail,args=[_,#b_literal{val=0}],dst=Dst}]=Is,
-            #b_br{bool=Dst,succ=Succ}=Last0, L, Map0, Acc) ->
-    case Map0 of
-        #{L := Bits} when is_integer(Bits) ->
-            %% The `bs_match` instruction targeting this block on failure
-            %% will only fail when the end of the binary has been reached.
-            %% There is no need for the test.
-            Last = beam_ssa:normalize(Last0#b_br{fail=Succ}),
-            {reverse(Acc, Is),Last,Map0};
-        #{} ->
-            {reverse(Acc, Is),Last0,Map0}
-    end;
-bsm_tail_is([#b_set{}=I|Is], Last, L, Map, Acc) ->
-    bsm_tail_is(Is, Last, L, Map, [I|Acc]);
-bsm_tail_is([], Last, _L, Map0, Acc) ->
-    Map = foldl(fun(F, A) ->
-                        A#{F => unknown}
-                end, Map0, beam_ssa:successors(#b_blk{is=[],last=Last})),
-    {reverse(Acc),Last,Map}.
-
-bsm_tail_num_matched([#b_literal{val=skip},Ctx,Type,Flags,Size,Unit], Map) ->
-    bsm_tail_num_matched([Type,Ctx,Flags,Size,Unit], Map);
-bsm_tail_num_matched([#b_literal{val=Type},Ctx,#b_literal{},
-                      #b_literal{val=Size},#b_literal{val=Unit}], Map)
+bsm_tail_is_1([#b_set{op=bs_match,anno=Anno,dst=Dst,args=[_, Ctx | _]=Args},
+               #b_set{op={succeeded,guard},dst=SuccDst,args=[Dst]}],
+              #b_blk{last=#b_br{bool=SuccDst,succ=Succ,fail=Fail}=Last}=Blk0,
+              L, Tags) ->
+    case Tags of
+        #{ L := Ctx } ->
+            %% This block can only be reached through `bs_match` instructions
+            %% that fail because the context is empty, so we KNOW that the
+            %% current instruction cannot match.
+            %%
+            %% Kill the instruction and propagate the condition.
+            Blk = Blk0#b_blk{last=beam_ssa:normalize(Last#b_br{succ=Fail})},
+            {Blk, bsm_tail_update_target(Fail, Fail, Ctx, Tags)};
+        #{ L := _ } ->
+            %% `any` or different context, mark the fail block with whether
+            %% it's reachable only because the context is empty.
+            Tag = bsm_tail_match_tag(Args, Anno),
+            {Blk0, bsm_tail_update_target(Succ, Fail, Tag, Tags)}
+    end;
+bsm_tail_is_1([#b_set{op=bs_test_tail,args=[Ctx,#b_literal{val=Size}],dst=Dst}],
+              #b_blk{last=#b_br{bool=Dst,succ=Succ,fail=Fail}=Last0}=Blk0,
+              L, Tags) ->
+    true = is_integer(Size) andalso Size >= 0,  %Assertion.
+    case Tags of
+        #{ L := Ctx } ->
+            %% This block can only be reached through `bs_match` instructions
+            %% that fail because the end of the context has been reached, so
+            %% the test is redundant.
+            Next = case Size of
+                       0 -> Succ;
+                       _ -> Fail
+                   end,
+            Last = beam_ssa:normalize(Last0#b_br{succ=Next,fail=Next}),
+            Blk = Blk0#b_blk{last=Last},
+            {Blk, bsm_tail_update_target(Next, Next, Ctx, Tags)};
+        #{ L := _ } ->
+            %% `any` or different context. We cannot optimize this, but it's
+            %% safe to mark the success block as only being reachable when the
+            %% context is empty.
+            Tag = case Size of
+                      0 -> Ctx;
+                      _ -> any
+                  end,
+            {Blk0, bsm_tail_update_target(Fail, Succ, Tag, Tags)}
+    end;
+bsm_tail_is_1([#b_set{} | Is], Blk, L, Tags) ->
+    bsm_tail_is_1(Is, Blk, L, Tags);
+bsm_tail_is_1([], Blk, _L, Tags0) ->
+    Tags = foldl(fun(Lbl, Acc) ->
+                         Acc#{ Lbl => any }
+                 end, Tags0, beam_ssa:successors(Blk)),
+    {Blk, Tags}.
+
+bsm_tail_match_tag([#b_literal{val=Type},
+                    #b_var{}=Ctx,
+                    #b_literal{},
+                    #b_literal{val=Size},
+                    #b_literal{val=Unit}],
+                   Anno)
   when (Type =:= integer orelse Type =:= binary),
        is_integer(Size), is_integer(Unit) ->
     Bits = Size * Unit,
-    case Map of
-        #{Ctx := Bits} when is_integer(Bits) ->
-            Bits;
+    case Anno of
+        #{ arg_types := #{ 1 := CtxType } } ->
+            case beam_types:get_bs_matchable_unit(CtxType) of
+                Bits -> Ctx;
+                _ -> any
+            end;
         #{} ->
-            unknown
-    end;
-bsm_tail_num_matched(_Args, _Map) ->
-    unknown.
+            any
+    end;
+bsm_tail_match_tag([#b_literal{val=skip}, Ctx, Type | Rest], Anno) ->
+    bsm_tail_match_tag([Type, Ctx | Rest], Anno);
+bsm_tail_match_tag(_Args, _Anno) ->
+    any.
+
+bsm_tail_update_target(Succ, Fail, Tag, Tags) when Succ =/= Fail ->
+    bsm_tail_update_target(Fail, Fail, Tag, Tags#{ Succ => any });
+bsm_tail_update_target(Same, Same, Tag, Tags) ->
+    case Tags of
+        #{ Same := Tag } -> Tags;
+        #{ Same := _ } -> Tags#{ Same => any };
+        #{} -> Tags#{ Same => Tag }
+    end.
 
 %%%
 %%% Optimize binary construction.
diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl
@@ -915,11 +915,13 @@ vi({test,bs_skip_bits2,{f,Fail},[Ctx,Size0,Unit,_Flags]}, Vst) ->
              end,
 
     validate_bs_skip(Fail, Ctx, Stride, Vst);
-vi({test,bs_test_tail2,{f,Fail},[Ctx,_Size]}, Vst) ->
+vi({test,bs_test_tail2,{f,Fail},[Ctx0,_Size]}, Vst) ->
+    Ctx = unpack_typed_arg(Ctx0, Vst),
     assert_no_exception(Fail),
     assert_type(#t_bs_context{}, Ctx, Vst),
     branch(Fail, Vst);
-vi({test,bs_test_unit,{f,Fail},[Ctx,Unit]}, Vst) ->
+vi({test,bs_test_unit,{f,Fail},[Ctx0,Unit]}, Vst) ->
+    Ctx = unpack_typed_arg(Ctx0, Vst),
     assert_type(#t_bs_context{}, Ctx, Vst),
 
     Type = #t_bs_context{tail_unit=Unit},
@@ -1849,7 +1851,8 @@ validate_bs_get_1(Fail, Ctx, Live, Vst, SuccFun) ->
 validate_bs_skip(Fail, Ctx, Stride, Vst) ->
     validate_bs_skip(Fail, Ctx, Stride, no_live, Vst).
 
-validate_bs_skip(Fail, Ctx, Stride, Live, Vst) ->
+validate_bs_skip(Fail, Ctx0, Stride, Live, Vst) ->
+    Ctx = unpack_typed_arg(Ctx0, Vst),
     assert_no_exception(Fail),
 
     assert_type(#t_bs_context{}, Ctx, Vst),
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
@@ -2533,6 +2533,8 @@ empty_matches(Config) when is_list(Config) ->
     {'EXIT',{function_clause,[_|_]}} = catch em_4(<<0:1>>, <<>>),
     {'EXIT',{function_clause,[_|_]}} = catch em_4(<<0:1>>, <<0:1>>),
 
+    ok = em_5(),
+
     ok.
 
 em_1(Bytes) ->
@@ -2560,6 +2562,62 @@ em_3_1(I) -> I.
 em_4(<<X:0, _:X>>, <<Y:0, _:Y>>) ->
     ok.
 
+%% GH-10047
+em_5() ->
+    A = id(<<1>>),
+    Empty = id(<<>>),
+
+    true = is_bitstring(A),
+    true = is_bitstring(Empty),
+
+    <<1,128>> = em_5_1(A),
+    <<0>> = em_5_1(id(Empty)),
+
+    true = is_binary(A),
+    true = is_binary(Empty),
+
+    ok = em_5_coverage_1(A),
+    ok = em_5_coverage_1(Empty),
+
+    ok = em_5_coverage_2(A),
+    ok = em_5_coverage_2(Empty),
+
+    ok = em_5_coverage_3(A),
+    ok = em_5_coverage_3(Empty),
+
+    ok.
+
+em_5_1(<<Chunk:7/bits>>) ->
+    <<Chunk:7/bits, 0:1>>;
+em_5_1(<<Chunk:1/bits>>) ->
+    <<Chunk:1/bits, 0:7>>;
+em_5_1(<<>>) ->
+    <<0:8>>;
+em_5_1(<<Chunk:7/bits, Rest/bits>>) ->
+    <<Chunk:7/bits, 1:1, (em_5_1(Rest))/bits>>.
+
+%% Improves coverage.
+em_5_coverage_1(<<_:8/bits, Rest/binary>>) ->
+    em_5_coverage_1(Rest);
+em_5_coverage_1(<<_:8/integer, _/binary>>) ->
+    unreachable;
+em_5_coverage_1(<<>>) ->
+    ok.
+
+em_5_coverage_2(<<_:8/bits, Rest/binary>>) ->
+    em_5_coverage_2(Rest);
+em_5_coverage_2(<<_:8/integer, _/binary>>) ->
+    unreachable;
+em_5_coverage_2(<<>>) ->
+    ok.
+
+em_5_coverage_3(<<_:8/bits, Rest/binary>>) ->
+    em_5_coverage_3(Rest);
+em_5_coverage_3(<<_>>) ->
+    unreachable;
+em_5_coverage_3(<<>>) ->
+    ok.
+
 %% beam_trim would sometimes crash when bs_start_match4 had {atom,resume} as
 %% its fail label.
 trim_bs_start_match_resume(Config) when is_list(Config) ->
