From e0065b11830dba9152dfa48d970d601d553f7fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 3 Aug 2023 06:45:30 +0200 Subject: [PATCH] beam_jump: Eliminate unsafe sharing of blocks Fixes #7477 --- lib/compiler/src/beam_jump.erl | 38 ++++++++++++++++++---------- lib/compiler/test/trycatch_SUITE.erl | 12 +++++++++ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/lib/compiler/src/beam_jump.erl b/lib/compiler/src/beam_jump.erl index 0ca75504cf8c..9536c3073db8 100644 --- a/lib/compiler/src/beam_jump.erl +++ b/lib/compiler/src/beam_jump.erl @@ -487,22 +487,26 @@ is_shareable([]) -> true. %% branches to them are located. %% %% If there is more than one scope in the function (that is, if there -%% try/catch or catch in the function), the scope identifiers will be -%% added to the line instructions. Recording the scope in the line -%% instructions makes beam_jump idempotent, ensuring that beam_jump -%% will not do any unsafe optimizations when when compiling from a .S -%% file. +%% is any try/catch or catch in the function), the scope identifiers +%% will be added to the line instructions. Recording the scope in the +%% line instructions makes beam_jump idempotent, ensuring that +%% beam_jump will not do any unsafe optimizations when compiling from +%% a .S file. %% classify_labels(Is) -> classify_labels(Is, 0, #{}). -classify_labels([{'catch',_,_}|Is], Scope, Safe) -> - classify_labels(Is, Scope+1, Safe); +classify_labels([{'catch',_,{f,L}}|Is], Scope0, Safe0) -> + Scope = Scope0 + 1, + Safe = classify_add_label(L, Scope, Safe0), + classify_labels(Is, Scope, Safe); classify_labels([{catch_end,_}|Is], Scope, Safe) -> classify_labels(Is, Scope+1, Safe); -classify_labels([{'try',_,_}|Is], Scope, Safe) -> - classify_labels(Is, Scope+1, Safe); +classify_labels([{'try',_,{f,L}}|Is], Scope0, Safe0) -> + Scope = Scope0 + 1, + Safe = classify_add_label(L, Scope, Safe0), + classify_labels(Is, Scope, Safe); classify_labels([{'try_end',_}|Is], Scope, Safe) -> classify_labels(Is, Scope+1, Safe); classify_labels([{'try_case',_}|Is], Scope, Safe) -> @@ -512,11 +516,7 @@ classify_labels([{'try_case_end',_}|Is], Scope, Safe) -> classify_labels([I|Is], Scope, Safe0) -> Labels = instr_labels(I), Safe = foldl(fun(L, A) -> - case A of - #{L := [Scope]} -> A; - #{L := Other} -> A#{L => ordsets:add_element(Scope, Other)}; - #{} -> A#{L => [Scope]} - end + classify_add_label(L, Scope, A) end, Safe0, Labels), classify_labels(Is, Scope, Safe); classify_labels([], Scope, Safe) -> @@ -529,6 +529,16 @@ classify_labels([], Scope, Safe) -> Safe end. +classify_add_label(L, Scope, Map) -> + case Map of + #{L := [Scope]} -> + Map; + #{L := [_|_]=Set} -> + Map#{L => ordsets:add_element(Scope, Set)}; + #{} -> + Map#{L => [Scope]} + end. + %% Eliminate all fallthroughs. Return the result reversed. eliminate_fallthroughs([{label,L}=Lbl|Is], [I|_]=Acc) -> diff --git a/lib/compiler/test/trycatch_SUITE.erl b/lib/compiler/test/trycatch_SUITE.erl index 676b7fffc7fa..4d8b511ee835 100644 --- a/lib/compiler/test/trycatch_SUITE.erl +++ b/lib/compiler/test/trycatch_SUITE.erl @@ -1126,6 +1126,8 @@ grab_bag(_Config) -> {'EXIT',_} = (catch grab_bag_3()), + true = grab_bag_4(), + ok. grab_bag_1(V) -> @@ -1180,6 +1182,16 @@ grab_bag_3() -> %% would not return two values as expected. end =:= (V0 = 42). +grab_bag_4() -> + try + erlang:yield() + after + %% beam_jump would do an unsafe sharing of blocks, resulting + %% in an ambiguous_catch_try_state diagnostic from beam_validator. + catch <<>> = size(catch ([_ | _] = ok)) + end. + + stacktrace(_Config) -> V = [make_ref()|self()], case ?MODULE:module_info(native) of