File 0227-Eliminate-confusing-case_clause-exception.patch of Package erlang
From fe244ad80c1b8621fc499f94cac758b90d063831 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Tue, 11 Jan 2022 08:53:09 +0100
Subject: [PATCH] Eliminate confusing `case_clause` exception
Consider this module:
-module(bug).
-export([foo/0]).
foo() ->
fun(a) -> ok end(b).
The call to the fun will always fail, which will be noted by
the compiler:
1> c(bug).
bug.erl:5:5: Warning: no clause will ever match
% 5| fun(a) -> ok end(b).
% | ^
{ok,bug}
What is unexpected is that the exception that is raised is not
a `function_clause` exception:
2> bug:foo().
** exception error: no case clause matching {b}
in function bug:foo/0 (bug.erl, line 5)
This confusing `case_clause` exception started to appear in OTP 24
because of 72675baaa9fd30 (#4545) that inlines funs that are
immediately used.
Before OTP 24, when the fun was not inlined, the exception would be:
2> bug:foo().
** exception error: no function clause matching bug:'-foo/0-fun-0-'(b) (bug.erl, line 5)
The reason that `function_clause` exceptions are rewritten to
`case_clause` exceptions in inlined code is to avoid the even more
confusing and misleading exception:
2> bug:foo().
** exception error: no function clause matching bug:foo(b) (bug.erl, line 5)
This is confusing because it seems that `foo/0` was called with one argument.
To reduce the confusion, this commmit ensures that `function_clause` exceptions in
inlined code remains `function_clause` exceptions, but with a generated name that
makes it clear that the `function_clause` exception occurred in a fun:
2> bug:foo().
** exception error: no function clause matching bug:'-foo/0-inlined-0-'(b) (bug.erl, line 5)
Fixes #5513
---
lib/compiler/src/beam_kernel_to_ssa.erl | 13 ++-
lib/compiler/src/beam_ssa_pre_codegen.erl | 128 ++++++++++++++++------
lib/compiler/src/beam_validator.erl | 13 ++-
lib/compiler/src/v3_core.erl | 2 +-
lib/compiler/src/v3_kernel.erl | 49 ++++++---
lib/compiler/test/beam_except_SUITE.erl | 8 ++
lib/compiler/test/bs_match_SUITE.erl | 7 +-
lib/compiler/test/guard_SUITE.erl | 3 +-
lib/compiler/test/inline_SUITE.erl | 2 +-
9 files changed, 165 insertions(+), 60 deletions(-)
diff --git a/lib/compiler/src/beam_kernel_to_ssa.erl b/lib/compiler/src/beam_kernel_to_ssa.erl
index c2e22f4b6d..7722078206 100644
--- a/lib/compiler/src/beam_kernel_to_ssa.erl
+++ b/lib/compiler/src/beam_kernel_to_ssa.erl
@@ -25,7 +25,7 @@
-export([module/2]).
-import(lists, [all/2,append/1,flatmap/2,foldl/3,
- keysort/2,mapfoldl/3,map/2,member/2,
+ keyfind/3,keysort/2,mapfoldl/3,map/2,member/2,
reverse/1,reverse/2,sort/1]).
-include("v3_kernel.hrl").
@@ -672,7 +672,7 @@ call_cg(Func, As, [#k_var{name=R}|MoreRs]=Rs, Le, St0) ->
%% Ordinary function call in a function body.
Args = ssa_args([Func|As], St0),
{Ret,St1} = new_ssa_var(R, St0),
- Call = #b_set{anno=line_anno(Le),op=call,dst=Ret,args=Args},
+ Call = #b_set{anno=call_anno(Le),op=call,dst=Ret,args=Args},
%% If this is a call to erlang:error(), MoreRs could be a
%% nonempty list of variables that each need a value.
@@ -685,9 +685,16 @@ call_cg(Func, As, [#k_var{name=R}|MoreRs]=Rs, Le, St0) ->
enter_cg(Func, As0, Le, St0) ->
As = ssa_args([Func|As0], St0),
{Ret,St} = new_ssa_var('@ssa_ret', St0),
- Call = #b_set{anno=line_anno(Le),op=call,dst=Ret,args=As},
+ Call = #b_set{anno=call_anno(Le),op=call,dst=Ret,args=As},
{[Call,#b_ret{arg=Ret}],St}.
+call_anno(Le) ->
+ Anno = line_anno(Le),
+ case keyfind(inlined, 1, Le) of
+ false -> Anno;
+ {inlined,NameArity} -> Anno#{inlined => NameArity}
+ end.
+
%% bif_cg(#k_bif{}, Le,State) -> {[Ainstr],State}.
%% Generate code for a guard BIF or primop.
diff --git a/lib/compiler/src/beam_ssa_pre_codegen.erl b/lib/compiler/src/beam_ssa_pre_codegen.erl
index 3567fb4210..f5ad81f760 100644
--- a/lib/compiler/src/beam_ssa_pre_codegen.erl
+++ b/lib/compiler/src/beam_ssa_pre_codegen.erl
@@ -72,7 +72,8 @@
-import(lists, [all/2,any/2,append/1,duplicate/2,
foldl/3,last/1,member/2,partition/2,
- reverse/1,reverse/2,sort/1,splitwith/2,zip/2]).
+ reverse/1,reverse/2,seq/2,sort/1,
+ splitwith/2,usort/1,zip/2]).
-spec module(beam_ssa:b_module(), [compile:option()]) ->
{'ok',beam_ssa:b_module()}.
@@ -80,7 +81,8 @@
module(#b_module{body=Fs0}=Module, Opts) ->
UseBSM3 = not proplists:get_bool(no_bsm3, Opts),
Ps = passes(Opts),
- Fs = functions(Fs0, Ps, UseBSM3),
+ Fs1 = functions(Fs0, Ps, UseBSM3),
+ Fs = fc_stubs(Fs1, Module),
{ok,Module#b_module{body=Fs}}.
functions([F|Fs], Ps, UseBSM3) ->
@@ -835,19 +837,21 @@ match_fail_instrs_1([{L,#b_blk{is=Is0}=Blk}|Bs], Arity, Blocks0) ->
match_fail_instrs_1([], _Arity, Blocks) -> Blocks.
match_fail_instrs_blk([#b_set{op=put_tuple,dst=Dst,
- args=[#b_literal{val=Tag},Val]},
+ args=[#b_literal{val=Tag}|Values]},
#b_set{op=call,
args=[#b_remote{mod=#b_literal{val=erlang},
name=#b_literal{val=error}},
Dst]}=Call|Is],
_Arity, Acc) ->
- match_fail_instr(Call, Tag, Val, Is, Acc);
+ match_fail_instr(Call, Tag, Values, Is, Acc);
match_fail_instrs_blk([#b_set{op=call,
args=[#b_remote{mod=#b_literal{val=erlang},
name=#b_literal{val=error}},
- #b_literal{val={Tag,Val}}]}=Call|Is],
- _Arity, Acc) ->
- match_fail_instr(Call, Tag, #b_literal{val=Val}, Is, Acc);
+ #b_literal{val=Tuple}]}=Call|Is],
+ _Arity, Acc) when tuple_size(Tuple) >= 1 ->
+ [Tag|Values0] = tuple_to_list(Tuple),
+ Values = [#b_literal{val=V} || V <- Values0],
+ match_fail_instr(Call, Tag, Values, Is, Acc);
match_fail_instrs_blk([#b_set{op=call,
args=[#b_remote{mod=#b_literal{val=erlang},
name=#b_literal{val=error}},
@@ -860,34 +864,29 @@ match_fail_instrs_blk([#b_set{op=call,anno=Anno,
name=#b_literal{val=error}},
#b_literal{val=function_clause},
Stk]}=Call],
- {Arity,Location}, Acc) ->
- case match_fail_stk(Stk, Acc, [], []) of
- {[_|_]=Vars,Is} when length(Vars) =:= Arity ->
- case maps:get(location, Anno, none) of
- Location ->
- I = Call#b_set{op=match_fail,
- args=[#b_literal{val=function_clause}|Vars]},
- Is ++ [I];
- _ ->
- %% erlang:error/2 has a different location than the
- %% func_info instruction at the beginning of the function
- %% (probably because of inlining). Keep the original call.
- reverse(Acc, [Call])
- end;
- _ ->
- %% Either the stacktrace could not be picked apart (for example,
- %% if the call to erlang:error/2 was handwritten) or the number
- %% of arguments in the stacktrace was different from the arity
- %% of the host function (because it is the implementation of a
- %% fun). Keep the original call.
- reverse(Acc, [Call])
- end;
+ Arity, Acc) ->
+ match_fail_fc(Anno, Call, Stk, Arity, Acc);
match_fail_instrs_blk([I|Is], Arity, Acc) ->
match_fail_instrs_blk(Is, Arity, [I|Acc]);
match_fail_instrs_blk(_, _, _) ->
none.
-match_fail_instr(Call, Tag, Val, Is, Acc) ->
+match_fail_instr(Call, function_clause, Values, Is, Acc) ->
+ case beam_ssa:get_anno(inlined, Call, none) of
+ none ->
+ %% If there is no `inlined` annotation, it implies that
+ %% the call to erlang:error/1 was handwritten.
+ none;
+ {Name,Arity} ->
+ %% A `function_clause` in inlined code. Convert it to
+ %% a call to a stub function that will raise a proper
+ %% `function_clause` exception. (The stub function will
+ %% be created later by fc_stubs/2.)
+ Target = #b_local{name=#b_literal{val=Name},arity=Arity},
+ I = Call#b_set{args=[Target|Values]},
+ reverse(Acc, [I|Is])
+ end;
+match_fail_instr(Call, Tag, [Val], Is, Acc) ->
Op = case Tag of
badmatch -> Tag;
case_clause -> case_end;
@@ -900,19 +899,84 @@ match_fail_instr(Call, Tag, Val, Is, Acc) ->
_ ->
I = Call#b_set{op=match_fail,args=[#b_literal{val=Op},Val]},
reverse(Acc, [I|Is])
+ end;
+match_fail_instr(_, _, _, _, _) -> none.
+
+match_fail_fc(Anno, Call, Stk, {Arity,Location}, Acc) ->
+ case match_fail_stk(Stk, Acc, [], []) of
+ {[_|_]=Vars,Is} when length(Vars) =:= Arity ->
+ case maps:get(location, Anno, none) of
+ Location ->
+ I = Call#b_set{op=match_fail,
+ args=[#b_literal{val=function_clause}|Vars]},
+ Is ++ [I];
+ _ ->
+ %% erlang:error/2 has a different location than
+ %% the func_info instruction at the beginning of
+ %% the function (probably because of
+ %% inlining). Keep the original call.
+ reverse(Acc, [Call])
+ end;
+ _ ->
+ %% Either the stacktrace could not be picked apart (for
+ %% example, if the call to erlang:error/2 was handwritten)
+ %% or the number of arguments in the stacktrace was
+ %% different from the arity of the host function (because
+ %% it is the implementation of a fun). Keep the original
+ %% call.
+ reverse(Acc, [Call])
end.
match_fail_stk(#b_var{}=V, [#b_set{op=put_list,dst=V,args=[H,T]}|Is], IAcc, VAcc) ->
match_fail_stk(T, Is, IAcc, [H|VAcc]);
match_fail_stk(#b_literal{val=[H|T]}, Is, IAcc, VAcc) ->
match_fail_stk(#b_literal{val=T}, Is, IAcc, [#b_literal{val=H}|VAcc]);
-match_fail_stk(#b_literal{val=[]}, [], IAcc, VAcc) ->
- {reverse(VAcc),IAcc};
+match_fail_stk(#b_literal{val=[]}, Is, IAcc, VAcc) ->
+ {reverse(VAcc),reverse(Is, IAcc)};
match_fail_stk(T, [#b_set{op=Op}=I|Is], IAcc, VAcc)
when Op =:= bs_get_tail; Op =:= bs_set_position ->
match_fail_stk(T, Is, [I|IAcc], VAcc);
match_fail_stk(_, _, _, _) -> none.
+%% Create stubs for `function_clause` exceptions generated by
+%% inlined code.
+fc_stubs(Fs, #b_module{name=Mod}) ->
+ Stubs0 = usort(find_fc_calls(Fs, [])),
+ Stubs = [begin
+ Seq = seq(0, Arity-1),
+ Args = [#b_var{name=V} || V <- Seq],
+ XRegs = [{x,V} || V <- Seq],
+ Ret = #b_var{name='@ssa_ret'},
+ Regs = maps:from_list([{Ret,{x,0}}|zip(Args, XRegs)]),
+ Anno = #{func_info => {Mod,Name,Arity},
+ location => Location,
+ parameter_info => #{},
+ registers => Regs},
+ Fc = #b_set{op=match_fail,dst=Ret,
+ args=[#b_literal{val=function_clause}|Args]},
+ Blk = #b_blk{is=[Fc],last=#b_ret{arg=Ret}},
+ #b_function{anno=Anno,args=Args,
+ bs=#{0 => Blk},
+ cnt=1}
+ end || {{Name,Arity},Location} <- Stubs0],
+ Fs ++ Stubs.
+
+find_fc_calls([#b_function{bs=Blocks}|Fs], Acc0) ->
+ F = fun(#b_set{anno=Anno,op=call}, A) ->
+ case Anno of
+ #{inlined := FA} ->
+ [{FA,maps:get(location, Anno, [])}|A];
+ #{} ->
+ A
+ end;
+ (_, A) ->
+ A
+ end,
+ Acc = beam_ssa:fold_instrs(F, maps:keys(Blocks), Acc0, Blocks),
+ find_fc_calls(Fs, Acc);
+find_fc_calls([], Acc) -> Acc.
+
+
%%%
%%% Fix tuples.
%%%
diff --git a/lib/compiler/src/beam_validator.erl b/lib/compiler/src/beam_validator.erl
index 125c01c6a6..adbddc7059 100644
--- a/lib/compiler/src/beam_validator.erl
+++ b/lib/compiler/src/beam_validator.erl
@@ -245,7 +245,8 @@ build_function_table([{function,Name,Arity,Entry,Code0}|Fs], Acc) ->
case Code of
[{label,Entry} | Is] ->
Info = #{ arity => Arity,
- parameter_info => find_parameter_info(Is, #{}) },
+ parameter_info => find_parameter_info(Is, #{}),
+ always_fails => always_fails(Is) },
build_function_table(Fs, Acc#{ Entry => Info });
_ ->
%% Something is seriously wrong. Ignore it for now.
@@ -262,6 +263,9 @@ find_parameter_info([{'%', _} | Is], Acc) ->
find_parameter_info(_, Acc) ->
Acc.
+always_fails([{jump,_}|_]) -> true;
+always_fails(_) -> false.
+
validate_1(Is, MFA0, Entry, Level, Ft) ->
{Offset, MFA, Header, Body} = extract_header(Is, MFA0, Entry, 1, []),
@@ -3078,6 +3082,13 @@ will_call_succeed(bs_init_writable, _Vst) ->
yes;
will_call_succeed(raw_raise, _Vst) ->
no;
+will_call_succeed({f,Lbl}, #vst{ft=Ft}) ->
+ case Ft of
+ #{Lbl := #{always_fails := true}} ->
+ no;
+ #{} ->
+ maybe
+ end;
will_call_succeed(_Call, _Vst) ->
maybe.
diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl
index 6bf87e131c..8e89c88318 100644
--- a/lib/compiler/src/v3_core.erl
+++ b/lib/compiler/src/v3_core.erl
@@ -264,7 +264,7 @@ body(Cs0, Name, Arity, St0) ->
Args = reverse(Args0), %Nicer order
{Cs1,St2} = clauses(Cs0, St1),
{Ps,St3} = new_vars(Arity, St2), %Need new variables here
- Fc = function_clause(Ps, Anno),
+ Fc = function_clause(Ps, FunAnno),
{#ifun{anno=#a{anno=FunAnno},id=[],vars=Args,clauses=Cs1,fc=Fc},St3}.
%% clause(Clause, State) -> {Cclause,State}.
diff --git a/lib/compiler/src/v3_kernel.erl b/lib/compiler/src/v3_kernel.erl
index aa3fc033eb..aa8a906ca7 100644
--- a/lib/compiler/src/v3_kernel.erl
+++ b/lib/compiler/src/v3_kernel.erl
@@ -405,36 +405,51 @@ letrec_goto([{#c_var{name={Label,0}},Cfail}], Cb, Sub0,
%% erlang:error/2.
translate_match_fail(Arg, Sub, Anno, St0) ->
- Cargs = case {cerl:data_type(Arg),cerl:data_es(Arg)} of
- {tuple,[#c_literal{val=function_clause}|As]} ->
- translate_fc_args(As, Sub, St0);
- {_,_} ->
- [Arg]
- end,
- {Kargs,Ap,St} = atomic_list(Cargs, Sub, St0),
+ {Cargs,ExtraAnno,St1} =
+ case {cerl:data_type(Arg),cerl:data_es(Arg)} of
+ {tuple,[#c_literal{val=function_clause}|As]} ->
+ translate_fc_args(As, Sub, Anno, St0);
+ {_,_} ->
+ {[Arg],[],St0}
+ end,
+ {Kargs,Ap,St} = atomic_list(Cargs, Sub, St1),
Ar = length(Cargs),
- Call = #k_call{anno=Anno,
+ Call = #k_call{anno=ExtraAnno++Anno,
op=#k_remote{mod=#k_literal{val=erlang},
name=#k_literal{val=error},
arity=Ar},args=Kargs},
{Call,Ap,St}.
-translate_fc_args(As, Sub, #kern{fargs=Fargs}) ->
+translate_fc_args(As, Sub, Anno, #kern{fargs=Fargs}=St0) ->
case same_args(As, Fargs, Sub) of
true ->
%% The arguments for the `function_clause` exception are
%% the arguments for the current function in the correct
%% order.
- [#c_literal{val=function_clause},cerl:make_list(As)];
+ {[#c_literal{val=function_clause},cerl:make_list(As)],
+ [],
+ St0};
false ->
%% The arguments in the `function_clause` exception don't
- %% match the arguments for the current function because
- %% of inlining. Keeping the `function_clause`
- %% exception reason would be confusing. Rewrite it to
- %% a `case_clause` exception with the arguments in a
- %% tuple.
- [cerl:c_tuple([#c_literal{val=case_clause},
- cerl:c_tuple(As)])]
+ %% match the arguments for the current function because of
+ %% inlining.
+ Args = [cerl:c_tuple([#c_literal{val=function_clause}|As])],
+ case keyfind(function, 1, Anno) of
+ false ->
+ %% This is probably a fun that has been inlined
+ %% by sys_core_fold.
+ {Name,St1} = new_fun_name("inlined", St0),
+ {Args,
+ [{inlined,{Name,length(As)}}],
+ St1};
+ {_,{Name0,Arity}} ->
+ %% This is function that has been inlined.
+ Name1 = ["-inlined-",Name0,"/",Arity,"-"],
+ Name = list_to_atom(lists:concat(Name1)),
+ {Args,
+ [{inlined,{Name,Arity}}],
+ St0}
+ end
end.
same_args([#c_var{name=Cv}|Vs], [#k_var{name=Kv}|As], Sub) ->
diff --git a/lib/compiler/test/beam_except_SUITE.erl b/lib/compiler/test/beam_except_SUITE.erl
index 95e5989081..2bc805be71 100644
--- a/lib/compiler/test/beam_except_SUITE.erl
+++ b/lib/compiler/test/beam_except_SUITE.erl
@@ -135,14 +135,18 @@ coverage(_) ->
{'EXIT',{function_clause,[{?MODULE,foobar,[[fail],1,2],
[{file,"fake.erl"},{line,16}]}|_]}} =
(catch foobar([fail], 1, 2)),
+
{'EXIT',{function_clause,[{?MODULE,fake_function_clause1,[{a,b},42.0],_}|_]}} =
(catch fake_function_clause1({a,b})),
{'EXIT',{function_clause,[{?MODULE,fake_function_clause2,[42|bad_tl],_}|_]}} =
(catch fake_function_clause2(42, bad_tl)),
+
{'EXIT',{function_clause,[{?MODULE,fake_function_clause3,[x,y],_}|_]}} =
(catch fake_function_clause3(42, id([x,y]))),
+ {'EXIT',{{function_clause,a,b,c}, _}} = catch fake_function_clause4(),
+
{'EXIT',{{badmatch,0.0},_}} = (catch coverage_1(id(42))),
{'EXIT',{badarith,_}} = (catch coverage_1(id(a))),
@@ -153,9 +157,13 @@ coverage_1(X) ->
true = 0 / X.
fake_function_clause1(A) -> error(function_clause, [A,42.0]).
+
fake_function_clause2(A, Tl) -> error(function_clause, [A|Tl]).
+
fake_function_clause3(_, Stk) -> error(function_clause, Stk).
+fake_function_clause4() -> error({function_clause,a,b,c}).
+
binary_construction_allocation(_Config) ->
ok = do_binary_construction_allocation("PUT"),
ok.
diff --git a/lib/compiler/test/bs_match_SUITE.erl b/lib/compiler/test/bs_match_SUITE.erl
index f7542be568..96ae549b28 100644
--- a/lib/compiler/test/bs_match_SUITE.erl
+++ b/lib/compiler/test/bs_match_SUITE.erl
@@ -1468,10 +1468,11 @@ haystack_2(Haystack) ->
fc({'EXIT',{function_clause,_}}) -> ok;
fc({'EXIT',{{case_clause,_},_}}) when ?MODULE =:= bs_match_inline_SUITE -> ok.
-fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,Name,Args,_}|_]}}) -> ok;
-fc(_, Args, {'EXIT',{{case_clause,ActualArgs},_}})
+fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,Name,Args,_}|_]}}) ->
+ ok;
+fc(Name, Args, {'EXIT',{function_clause,[{?MODULE,_,Args,_}|_]}})
when ?MODULE =:= bs_match_inline_SUITE ->
- Args = tuple_to_list(ActualArgs).
+ ok.
%% Cover the clause handling bs_context to binary in
%% beam_block:initialized_regs/2.
diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl
index 6699d82262..6a42453654 100644
--- a/lib/compiler/test/guard_SUITE.erl
+++ b/lib/compiler/test/guard_SUITE.erl
@@ -3070,5 +3070,4 @@ check(F, Result) ->
ct:fail(check_failed)
end.
-fc({'EXIT',{function_clause,_}}) -> ok;
-fc({'EXIT',{{case_clause,_},_}}) when ?MODULE =:= guard_inline_SUITE -> ok.
+fc({'EXIT',{function_clause,_}}) -> ok.
diff --git a/lib/compiler/test/inline_SUITE.erl b/lib/compiler/test/inline_SUITE.erl
index 7482f53ed4..b2778bebcd 100644
--- a/lib/compiler/test/inline_SUITE.erl
+++ b/lib/compiler/test/inline_SUITE.erl
@@ -332,7 +332,7 @@ badarg(Reply, _A) ->
Reply.
otp_7223(Config) when is_list(Config) ->
- {'EXIT', {{case_clause,{1}},_}} = (catch otp_7223_1(1)),
+ {'EXIT', {function_clause, [{?MODULE,_,[1],_}|_]}} = (catch otp_7223_1(1)),
ok.
-compile({inline,[{otp_7223_1,1}]}).
--
2.31.1
