Skip to content

Commit da168c7

Browse files
committed
compiler: Fix inplace tuple updates for an edge case
Fix #10367. This reverts a30e40e. In certain edge cases, aggregates are incorrectly marked as unique when their contained records are not unique. This leads to unsafe destructive updates and strange runtime behaviours. If there is a less conservative fix that is more specific for this edge case, we can revisit.
1 parent d196779 commit da168c7

File tree

3 files changed

+34
-23
lines changed

3 files changed

+34
-23
lines changed

lib/compiler/src/beam_ssa_ss.erl

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -645,12 +645,7 @@ set_alias([], State) ->
645645
State.
646646

647647
get_alias_edges(V, State) ->
648-
OutEdges = [To
649-
|| {#b_var{},To,Kind} <- beam_digraph:out_edges(State, V),
650-
case Kind of
651-
{embed,_} -> false;
652-
_ -> true
653-
end],
648+
OutEdges = [To || {#b_var{},To,_} <- beam_digraph:out_edges(State, V)],
654649
EmbedEdges = [Src
655650
|| {#b_var{}=Src,_,Lbl} <- beam_digraph:in_edges(State, V),
656651
case Lbl of

lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -831,13 +831,12 @@ aliased_pair_tl_instr(Ls) ->
831831
aliasing_after_tuple_extract(N) ->
832832
aliasing_after_tuple_extract(N, {<<>>, dummy}).
833833

834-
%% Check that the Acc tuple is unique on entry, but that the elements
835-
%% are aliased.
834+
%% Check that both the tuple (Acc) and the extracted element (X) are
835+
%% aliased.
836836
aliasing_after_tuple_extract(0, Acc) ->
837837
%ssa% (_,Acc) when post_ssa_opt ->
838-
%ssa% X = get_tuple_element(Acc, 0) {unique => [Acc]},
839-
%ssa% Bin = bs_create_bin(_,_,X,...) {aliased => [X]},
840-
%ssa% Tuple = put_tuple(Bin, Acc) {aliased => [Bin], unique => [Acc]}.
838+
%ssa% X = get_tuple_element(Acc, 0) {aliased => [Acc]},
839+
%ssa% _ = bs_create_bin(_,_,X,...) {aliased => [X]}.
841840
Acc;
842841
aliasing_after_tuple_extract(N, Acc) ->
843842
{X,_} = Acc,
@@ -853,9 +852,8 @@ alias_after_pair_hd(0, Acc) ->
853852
Acc;
854853
alias_after_pair_hd(N, Acc) ->
855854
%ssa% (_,Acc) when post_ssa_opt ->
856-
%ssa% X = get_hd(Acc) {unique => [Acc]},
857-
%ssa% Bin = bs_create_bin(_,_,X,...) {aliased => [X]},
858-
%ssa% Tuple = put_list(Bin, Acc) {aliased => [Bin], unique => [Acc]}.
855+
%ssa% X = get_hd(Acc) {aliased => [Acc]},
856+
%ssa% _ = bs_create_bin(_,_,X,...) {aliased => [X]}.
859857
[X|_] = Acc,
860858
alias_after_pair_hd(N - 1, [<<X/bitstring, 1>>|Acc]).
861859

@@ -868,9 +866,8 @@ alias_after_pair_tl(0, Acc) ->
868866
Acc;
869867
alias_after_pair_tl(N, Acc) ->
870868
%ssa% (_,Acc) when post_ssa_opt ->
871-
%ssa% X = get_tl(Acc) {unique => [Acc]},
872-
%ssa% Bin = bs_create_bin(_,_,X,...) {aliased => [X]},
873-
%ssa% Tuple = put_list(Acc, Bin) {aliased => [Bin], unique => [Acc]}.
869+
%ssa% X = get_tl(Acc) {aliased => [Acc]},
870+
%ssa% _ = bs_create_bin(_,_,X,...) {aliased => [X]}.
874871
[_|X] = Acc,
875872
alias_after_pair_tl(N - 1, [Acc|<<X/bitstring, 1>>]).
876873

@@ -1204,8 +1201,8 @@ nested_tuple_inner() ->
12041201

12051202
nested_tuple() ->
12061203
%ssa% () when post_ssa_opt ->
1207-
%ssa% U = bs_create_bin(append, _, T, ...) { unique => [T] },
1208-
%ssa% R = put_tuple(U, A) { aliased => [A], unique => [U] },
1204+
%ssa% U = bs_create_bin(append, _, T, ...) { aliased => [T] },
1205+
%ssa% R = put_tuple(U, A) { aliased => [U, A] },
12091206
%ssa% ret(R).
12101207
{{{{Z,X}}}} = nested_tuple_inner(),
12111208
{<<Z/binary, 1:8>>,X}.
@@ -1228,8 +1225,8 @@ nested_mixed_inner() ->
12281225

12291226
nested_mixed() ->
12301227
%ssa% () when post_ssa_opt ->
1231-
%ssa% U = bs_create_bin(append, _, T, ...) { unique => [T] },
1232-
%ssa% R = put_tuple(U, A) { aliased => [A], unique => [U] },
1228+
%ssa% U = bs_create_bin(append, _, T, ...) { aliased => [T] },
1229+
%ssa% R = put_tuple(U, A) { aliased => [U, A] },
12331230
%ssa% ret(R).
12341231
[{[{Z,X}]}] = nested_mixed_inner(),
12351232
{<<Z/binary, 1:8>>,X}.

lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
-export([do0a/0, do0b/2, different_sizes/2, ambiguous_inits/1,
2626
update_record0/0, fc/0, track_update_record/1,
2727
gh8124_a/0, gh8124_b/0, tuple_set_a/1, tuple_set_b/0,
28-
failure_to_patch_list/0, erierl1208/0, gh_9903/0]).
28+
failure_to_patch_list/0, erierl1208/0, gh_9903/0,
29+
gh10367/0]).
2930
-record(r, {a=0,b=0,c=0,tot=0}).
3031
-record(r1, {a}).
3132
-record(r2, {b}).
@@ -279,7 +280,7 @@ tuple_set_a(Something) ->
279280

280281
tuple_set_b() ->
281282
%ssa% () when post_ssa_opt ->
282-
%ssa% _ = update_record(inplace, 2, _, ...).
283+
%ssa% _ = update_record(copy, 2, _, ...).
283284
case tuple_set_a(ex:f()) of
284285
{ok, A} ->
285286
case e:f() of
@@ -315,3 +316,21 @@ ftpl(Ts0) ->
315316
%ssa% _ = update_record(inplace, 5, X,...).
316317
A = erlang:timestamp(),
317318
Ts0#r{a=A}.
319+
320+
gh10367_gen() ->
321+
[#r4{a = a}, #r4{a = b, b = dict:new()}].
322+
323+
gh10367_update([_, #r4{a = b} = P2]) ->
324+
%ssa% (X) when post_ssa_opt ->
325+
%ssa% _ = update_record(copy, 3, _, ...).
326+
P2#r4{a = a};
327+
gh10367_update([_, #r4{a = a} = P2]) ->
328+
P2#r4{a = b}.
329+
330+
gh10367() ->
331+
%ssa% () when post_ssa_opt ->
332+
%ssa% _ = update_record(reuse, 3, _, ...).
333+
[P1, P2] = gh10367_gen(),
334+
Expected = P2#r4{a = a},
335+
timer:sleep(0),
336+
Expected = gh10367_update([P1, P2]).

0 commit comments

Comments
 (0)