From 640339f43a743b3bc4e6533194a659fbb717a3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Mon, 18 Mar 2024 12:43:47 +0100 Subject: [PATCH 1/2] debugger: Fix incorrect evaluation of nested records in guard Closes #8120 --- lib/debugger/src/dbg_ieval.erl | 17 +++++++++++++++++ lib/debugger/test/record_SUITE.erl | 29 +++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/debugger/src/dbg_ieval.erl b/lib/debugger/src/dbg_ieval.erl index 2d736898d897..e2dc2e2d317b 100644 --- a/lib/debugger/src/dbg_ieval.erl +++ b/lib/debugger/src/dbg_ieval.erl @@ -1463,6 +1463,9 @@ guard_expr({'orelse',_,E1,E2}, Bs) -> {value,_Val}=Res -> Res end end; +guard_expr({'case',_,E0,Cs}, Bs) -> + {value,E} = guard_expr(E0, Bs), + guard_case_clauses(E, Cs, Bs); guard_expr({dbg,_,self,[]}, _) -> {value,get(self)}; guard_expr({safe_bif,_,erlang,'not',As0}, Bs) -> @@ -1505,6 +1508,20 @@ guard_expr({bin,_,Flds}, Bs) -> end), {value,V}. +%% guard_case_clauses(Value, Clauses, Bindings, Error, Ieval) +%% Error = try_clause | case_clause +guard_case_clauses(Val, [{clause,_,[P],G,B}|Cs], Bs0) -> + case match(P, Val, Bs0) of + {match,Bs} -> + case guard(G, Bs) of + true -> + guard_expr(hd(B), Bs); + false -> + guard_case_clauses(Val, Cs, Bs0) + end; + nomatch -> + guard_case_clauses(Val, Cs, Bs0) + end. %% eval_map_fields([Field], Bindings, IEvalState) -> %% {[{map_assoc | map_exact,Key,Value}],Bindings} diff --git a/lib/debugger/test/record_SUITE.erl b/lib/debugger/test/record_SUITE.erl index 2f75dcda4245..e3d66804bbc8 100644 --- a/lib/debugger/test/record_SUITE.erl +++ b/lib/debugger/test/record_SUITE.erl @@ -28,7 +28,8 @@ -export([all/0, suite/0,groups/0,init_per_group/2,end_per_group/2, init_per_testcase/2,end_per_testcase/2, init_per_suite/1,end_per_suite/1, - errors/1,record_test/1,eval_once/1]). + errors/1,record_test/1,eval_once/1, + nested_in_guard/1]). -export([debug/0]). @@ -50,7 +51,7 @@ end_per_group(_GroupName, Config) -> cases() -> - [errors, record_test, eval_once]. + [errors, record_test, eval_once, nested_in_guard]. init_per_testcase(_Case, Config) -> test_lib:interpret(?MODULE), @@ -295,6 +296,30 @@ once(Test, Record) -> end, Result. +nested_in_guard(_Config) -> + B = #bar{d = []}, + F = #foo{a = B}, + + ok = do_nested_in_guard(#foo{a=#bar{d=[]}}), + not_ok = do_nested_in_guard(#foo{a=#bar{}}), + not_ok = do_nested_in_guard(#foo{a={no_bar,a,b,c,d}}), + not_ok = do_nested_in_guard(#foo{a={bar,a}}), + not_ok = do_nested_in_guard(42), + not_ok = do_nested_in_guard(#bar{}), + not_ok = do_nested_in_guard([]), + + ok. + +-define(is_foo(X), (((X#foo.a)#bar.d == []))). + +do_nested_in_guard(F) -> + if + ?is_foo(F) -> + ok; + true -> + not_ok + end. + id(I) -> I. From d0dc1ebc5da30f67cd8a8f9abe5f5a2c8742f681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Wed, 20 Mar 2024 08:24:03 +0100 Subject: [PATCH 2/2] Fix failing binary match Closes #8280 --- lib/compiler/src/beam_ssa_codegen.erl | 4 ++++ lib/compiler/src/beam_ssa_opt.erl | 13 +++++++++++-- lib/compiler/test/bs_match_SUITE.erl | 8 ++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/compiler/src/beam_ssa_codegen.erl b/lib/compiler/src/beam_ssa_codegen.erl index 6f6de9996268..7879861c097e 100644 --- a/lib/compiler/src/beam_ssa_codegen.erl +++ b/lib/compiler/src/beam_ssa_codegen.erl @@ -2260,6 +2260,10 @@ bs_translate([]) -> []. bs_translate_collect([I|Is]=Is0, Ctx, Fail, Acc) -> case bs_translate_instr(I) of + {Ctx,_,{ensure_at_least,_,_}} -> + %% There should only be a single `ensure_at_least` + %% instruction in each `bs_match` instruction. + {bs_translate_fixup(Acc),Fail,Is0}; {Ctx,Fail,Instr} -> bs_translate_collect(Is, Ctx, Fail, [Instr|Acc]); {Ctx,{f,0},Instr} -> diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl index 13e4114319db..0be627760535 100644 --- a/lib/compiler/src/beam_ssa_opt.erl +++ b/lib/compiler/src/beam_ssa_opt.erl @@ -304,8 +304,8 @@ late_epilogue_passes(Opts) -> ?PASS(ssa_opt_sink), ?PASS(ssa_opt_blockify), ?PASS(ssa_opt_redundant_br), - ?PASS(ssa_opt_bs_ensure), ?PASS(ssa_opt_merge_blocks), + ?PASS(ssa_opt_bs_ensure), ?PASS(ssa_opt_try), ?PASS(ssa_opt_get_tuple_element), ?PASS(ssa_opt_tail_literals), @@ -3325,7 +3325,7 @@ redundant_br_safe_bool(Is, Bool) -> end. %%% -%%% Add the bs_ensure instruction before a sequence of `bs_match` +%%% Add the `bs_ensure` instruction before a sequence of `bs_match` %%% (SSA) instructions, each having a literal size and the %%% same failure label. %%% @@ -3333,6 +3333,15 @@ redundant_br_safe_bool(Is, Bool) -> %%% instruction that can match multiple segments having the same %%% failure label. %%% +%%% It is beneficial but not essential to run this pass after +%%% the `merge_blocks/1` pass. For the following example, two separate +%%% `bs_match/1` instructions will emitted if blocks have not been +%%% merged before this pass: +%%% +%%% A = 0, +%%% B = <<1, 2, 3>>, +%%% <> = <<0, 1, 2, 3>> +%%% ssa_opt_bs_ensure({#opt_st{ssa=Blocks0,cnt=Count0}=St, FuncDb}) when is_map(Blocks0) -> RPO = beam_ssa:rpo(Blocks0), diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl index b24f1afdd8e5..45c3ab04a9e5 100644 --- a/lib/compiler/test/bs_match_SUITE.erl +++ b/lib/compiler/test/bs_match_SUITE.erl @@ -2724,6 +2724,8 @@ bs_match(_Config) -> {'EXIT',{{badmatch,<<>>},_}} = catch do_bs_match_gh_7467(<<>>), + {0,<<1,2,3>>} = do_bs_match_gh_8280(), + ok. do_bs_match_1(_, X) -> @@ -2798,6 +2800,12 @@ do_bs_match_gh_6755(B) -> do_bs_match_gh_7467(A) -> do_bs_match_gh_7467(<<_:1/bits>> = A). +do_bs_match_gh_8280() -> + A = 0, + B = <<1, 2, 3>>, + <> = id(<<0, 1, 2, 3>>), + {A, B}. + %% GH-6348/OTP-18297: Allow aliases for binaries. -record(ba_foo, {a,b,c}).