From 434f8b4f61953a5140367a8934251107403dacb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Thu, 3 Oct 2024 09:51:50 +0200 Subject: [PATCH] Eliminate unsafe fusion of BEAM instructions Closes #8875 --- erts/emulator/beam/jit/arm/ops.tab | 4 ++-- erts/emulator/beam/jit/x86/ops.tab | 2 +- erts/emulator/test/tuple_SUITE.erl | 38 ++++++++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/erts/emulator/beam/jit/arm/ops.tab b/erts/emulator/beam/jit/arm/ops.tab index e7fe14477251..73266ec4a0a0 100644 --- a/erts/emulator/beam/jit/arm/ops.tab +++ b/erts/emulator/beam/jit/arm/ops.tab @@ -210,11 +210,11 @@ current_tuple/1 current_tuple/2 is_tuple Fail=f Src | test_arity Fail2 Src2 Arity | - equal(Fail, Fail2) | equal(Src, Src) => + equal(Fail, Fail2) | equal(Src, Src2) => i_is_tuple_of_arity Fail Src Arity | current_tuple Src is_tuple Fail1=f Src | test_arity Fail2 Src2 Arity | - equal(Src, Src) => + equal(Src, Src2) => i_is_tuple_of_arity_ff Fail1 Fail2 Src Arity | current_tuple Src test_arity Fail Src Arity => i_test_arity Fail Src Arity | current_tuple Src diff --git a/erts/emulator/beam/jit/x86/ops.tab b/erts/emulator/beam/jit/x86/ops.tab index d73435be65b7..f8a46e145334 100644 --- a/erts/emulator/beam/jit/x86/ops.tab +++ b/erts/emulator/beam/jit/x86/ops.tab @@ -218,7 +218,7 @@ is_tuple Fail=f Src | test_arity Fail2 Src2 Arity | i_is_tuple_of_arity Fail Src Arity | current_tuple Src is_tuple Fail1=f Src | test_arity Fail2 Src2 Arity | - equal(Src, Src) => + equal(Src, Src2) => i_is_tuple_of_arity_ff Fail1 Fail2 Src Arity | current_tuple Src test_arity Fail Src Arity => i_test_arity Fail Src Arity | current_tuple Src diff --git a/erts/emulator/test/tuple_SUITE.erl b/erts/emulator/test/tuple_SUITE.erl index 11967b78f368..ae4c59e7b680 100644 --- a/erts/emulator/test/tuple_SUITE.erl +++ b/erts/emulator/test/tuple_SUITE.erl @@ -27,7 +27,8 @@ t_append_element/1, t_append_element_upper_boundry/1, build_and_match/1, tuple_with_case/1, tuple_in_guard/1, get_two_tuple_elements/1, - record_update/1]). + record_update/1, + bad_tuple_match/1]). -include_lib("common_test/include/ct.hrl"). %% Tests tuples and the BIFs: @@ -51,7 +52,8 @@ all() -> t_insert_element, t_delete_element, tuple_with_case, tuple_in_guard, get_two_tuple_elements, - record_update]. + record_update, + bad_tuple_match]. groups() -> []. @@ -701,6 +703,38 @@ do_record_update_3(L, N, R0) -> R end. +%% GH-8875 +bad_tuple_match(_Config) -> + ContentName = id(~"content-name"), + Content = id(#{ContentName => value}), + + Find = case maps:find(ContentName, Content) of + {ok, _} -> {ok1, aa}; + error -> ok + end, + + Details = try id(good) of + good -> + {ok2, bb, cc} + catch + _:_ -> + {error, some_reason} + end, + + %% Compiler optimizations will turn the following two lines into: + %% + %% {test,is_tuple,Fail1,[{y,0}]} + %% {test,test_arity,Fail2,[{x,0},3]}. + %% + %% The JIT would rewrite that to: + %% + %% {i_is_tuple_of_arity_ff,Fail1,Fail2,{y,0},3} + %% + %% That is unsafe because the source tuples are distinct. + {ok1, _} = Find, + {ok2, _, _} = Details, + + ok. %% Use this function to avoid compile-time evaluation of an expression. id(I) -> I.