aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Högberg <[email protected]>2019-03-25 19:57:33 +0100
committerJohn Högberg <[email protected]>2019-03-26 12:15:55 +0100
commita5976a6a6065e0d5c0c3ca118f50494c10fdc5a7 (patch)
tree3be7ed12f67d3dce505f26a9cf8b280051afbecd
parentedb8aa4f76981c1f32c9b429f47ce30100126e58 (diff)
downloadotp-a5976a6a6065e0d5c0c3ca118f50494c10fdc5a7.tar.gz
otp-a5976a6a6065e0d5c0c3ca118f50494c10fdc5a7.tar.bz2
otp-a5976a6a6065e0d5c0c3ca118f50494c10fdc5a7.zip
compiler: Fully disable no_return optimizations in try blocks
Validation could fail when a function that never returned was used in a try block (see attached test case). It's possible to solve this without disabling the optimization as the generated code is sound, but I'm not comfortable making such a large change this close to the OTP 22 release.
-rw-r--r--lib/compiler/src/beam_ssa_type.erl23
-rw-r--r--lib/compiler/test/trycatch_SUITE.erl27
2 files changed, 46 insertions, 4 deletions
diff --git a/lib/compiler/src/beam_ssa_type.erl b/lib/compiler/src/beam_ssa_type.erl
index c01ea4af91..06b42f1928 100644
--- a/lib/compiler/src/beam_ssa_type.erl
+++ b/lib/compiler/src/beam_ssa_type.erl
@@ -267,10 +267,29 @@ opt_is([#b_set{op=call,args=Args0,dst=Dst}=I0|Is],
I1 = beam_ssa:normalize(I0#b_set{args=Args}),
{Ts1,Ds,Fdb,I2} = opt_call(I1, D, Ts0, Ds0, Fdb0),
case {map_get(Dst, Ts1),Is} of
- {_,[#b_set{op=succeeded}]} ->
+ {Type,[#b_set{op=succeeded}]} when Type =/= none ->
%% This call instruction is inside a try/catch
- %% block. Don't attempt to optimize it.
+ %% block. Don't attempt to simplify it.
opt_is(Is, Ts1, Ds, Fdb, D, Sub0, [I2|Acc]);
+ {none,[#b_set{op=succeeded}]} ->
+ %% This call instruction is inside a try/catch
+ %% block, but we know it will never return and
+ %% later optimizations may try to exploit that.
+ %%
+ %% For example, if we have an expression that
+ %% either returns this call or a tuple, we know
+ %% that the expression always returns a tuple
+ %% and can turn a later element/3 into
+ %% get_tuple_element.
+ %%
+ %% This is sound but difficult to validate in a
+ %% meaningful way as try/catch currently forces
+ %% us to maintain the illusion that the success
+ %% block is reachable even when its not, so we
+ %% disable the optimization to keep things
+ %% simple.
+ Ts = Ts1#{ Dst := any },
+ opt_is(Is, Ts, Ds, Fdb, D, Sub0, [I2|Acc]);
{none,_} ->
%% This call never returns. The rest of the
%% instructions will not be executed.
diff --git a/lib/compiler/test/trycatch_SUITE.erl b/lib/compiler/test/trycatch_SUITE.erl
index 8f9cd9ab1e..539f9d69fa 100644
--- a/lib/compiler/test/trycatch_SUITE.erl
+++ b/lib/compiler/test/trycatch_SUITE.erl
@@ -27,7 +27,8 @@
nested_horrid/1,last_call_optimization/1,bool/1,
plain_catch_coverage/1,andalso_orelse/1,get_in_try/1,
hockey/1,handle_info/1,catch_in_catch/1,grab_bag/1,
- stacktrace/1,nested_stacktrace/1,raise/1]).
+ stacktrace/1,nested_stacktrace/1,raise/1,
+ no_return_in_try_block/1]).
-include_lib("common_test/include/ct.hrl").
@@ -43,7 +44,8 @@ groups() ->
nested_after,nested_horrid,last_call_optimization,
bool,plain_catch_coverage,andalso_orelse,get_in_try,
hockey,handle_info,catch_in_catch,grab_bag,
- stacktrace,nested_stacktrace,raise]}].
+ stacktrace,nested_stacktrace,raise,
+ no_return_in_try_block]}].
init_per_suite(Config) ->
@@ -1287,5 +1289,26 @@ do_test_raise_4(Expr) ->
erlang:raise(exit, {exception,C,E,Stk}, Stk)
end.
+no_return_in_try_block(Config) when is_list(Config) ->
+ 1.0 = no_return_in_try_block_1(0),
+ 1.0 = no_return_in_try_block_1(0.0),
+
+ gurka = no_return_in_try_block_1(gurka),
+ [] = no_return_in_try_block_1([]),
+
+ ok.
+
+no_return_in_try_block_1(H) ->
+ try
+ Float = if
+ is_number(H) -> float(H);
+ true -> no_return()
+ end,
+ Float + 1
+ catch
+ throw:no_return -> H
+ end.
+
+no_return() -> throw(no_return).
id(I) -> I.