Skip to content

Commit

Permalink
compiler: Avoid crash in private-append optimization
Browse files Browse the repository at this point in the history
Avoid a crash in the private-append optimization due to attempting to
rewrite a literal which isn't a bitstring.

As value tracking is done without type information, it can happen that
when following def chains, we encounter literals which are not
bitstrings. We should never schedule a literal which isn't a bitstring
for rewriting when the complete literal is what we are tracking,
i.e. when the element is `self`.

Closes erlang#6847
  • Loading branch information
frej committed Feb 15, 2023
1 parent 2b397d7 commit 1752215
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
7 changes: 7 additions & 0 deletions lib/compiler/src/beam_ssa_private_append.erl
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ get_results(SSA, Element, Fun, DefSt) ->
get_results([{_,#b_blk{last=#b_ret{arg=#b_var{}=V}}}|Rest],
Acc, Element, Fun, DefSt) ->
get_results(Rest, [{V,Element}|Acc], Element, Fun, DefSt);
get_results([{_,#b_blk{last=#b_ret{arg=#b_literal{val=Lit}}}}|Rest],
Acc, Element=self, Fun, DefSt) when not is_bitstring(Lit) ->
%% As value tracking is done without type information, we can
%% follow def chains which don't terminate in a bitstring. This is
%% harmless, but we should ignore them and not, later on, try to
%% patch them to a bs_writable_binary.
get_results(Rest, Acc, Element, Fun, DefSt);
get_results([{Lbl,#b_blk{last=#b_ret{arg=#b_literal{}}}}|Rest],
Acc, Element, Fun, DefSt0) ->
DefSt = add_literal(Fun, {ret,Lbl,Element}, DefSt0),
Expand Down
16 changes: 16 additions & 0 deletions lib/compiler/test/beam_ssa_check_SUITE_data/private_append.erl
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
transformable30/1,
transformable31a/1,
transformable31b/1,
transformable32/0,
transformable32/1,

not_transformable1/2,
not_transformable2/1,
Expand Down Expand Up @@ -748,6 +750,20 @@ transformable31([], Acc, a) when is_binary(Acc) ->
transformable31([], Acc, b) when is_tuple(Acc) ->
<<>>.

%% Check that we don't crash (Github issue #6847) while attempting to
%% patch the empty list, but also that the literal <<>> becomes a
%% bs_init_writable.
transformable32() ->
<<(transformable32(ok))/binary>>.

transformable32(#{}) ->
%ssa% (_) when post_ssa_opt ->
%ssa% A = bs_init_writable(_),
%ssa% ret(A).
[];
transformable32(_) ->
<<>>.

% Should not be transformed as we can't know the alias status of Acc
not_transformable1([H|T], Acc) ->
%ssa% (_, Arg1) when post_ssa_opt ->
Expand Down

0 comments on commit 1752215

Please sign in to comment.