From 773e3d0f4a2d978a7ec6be9ed639eb1941d92d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 30 Jul 2019 08:34:51 +0200 Subject: Fix compiler crash when compiling with +no_type_opt If the `no_type_opt` option was given, the compiler would crash when attempting to compile containing with a `try`...`after` construct, such as this code: foo() -> try make_ref() after ok end. To avoid having this bug re-appear, test the `no_type_opt` option in the test suites. https://bugs.erlang.org/browse/ERL-997 --- lib/compiler/src/beam_ssa_opt.erl | 18 +++++++++++++++--- lib/compiler/src/compile.erl | 7 +++++-- lib/compiler/test/Makefile | 15 +++++++++++++-- lib/compiler/test/test_lib.erl | 3 ++- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/compiler/src/beam_ssa_opt.erl b/lib/compiler/src/beam_ssa_opt.erl index 229edc6a1d..d87c66c272 100644 --- a/lib/compiler/src/beam_ssa_opt.erl +++ b/lib/compiler/src/beam_ssa_opt.erl @@ -1939,12 +1939,24 @@ verify_merge_is(_) -> is_merge_allowed(_, #b_blk{}, #b_blk{is=[#b_set{op=peek_message}|_]}) -> false; -is_merge_allowed(L, #b_blk{last=#b_br{}}=Blk, #b_blk{}) -> +is_merge_allowed(L, #b_blk{last=#b_br{}}=Blk, #b_blk{is=Is}) -> %% The predecessor block must have exactly one successor (L) for %% the merge to be safe. case beam_ssa:successors(Blk) of - [L] -> true; - [_|_] -> false + [L] -> + case Is of + [#b_set{op=phi,args=[_]}|_] -> + %% The type optimizer pass must have been + %% turned off, since it would have removed this + %% redundant phi node. Refuse to merge the blocks + %% to ensure that this phi node remains at the + %% beginning of a block. + false; + _ -> + true + end; + [_|_] -> + false end; is_merge_allowed(_, #b_blk{last=#b_switch{}}, #b_blk{}) -> false. diff --git a/lib/compiler/src/compile.erl b/lib/compiler/src/compile.erl index 28db8986ff..0325c714d0 100644 --- a/lib/compiler/src/compile.erl +++ b/lib/compiler/src/compile.erl @@ -268,8 +268,11 @@ expand_opt(r21, Os) -> [no_put_tuple2 | expand_opt(no_bsm3, Os)]; expand_opt({debug_info_key,_}=O, Os) -> [encrypt_debug_info,O|Os]; -expand_opt(no_type_opt, Os) -> - [no_ssa_opt_type_start, +expand_opt(no_type_opt=O, Os) -> + %% Be sure to keep the no_type_opt option so that it will + %% be recorded in the BEAM file, allowing the test suites + %% to recompile the file with this option. + [O,no_ssa_opt_type_start, no_ssa_opt_type_continue, no_ssa_opt_type_finish | Os]; expand_opt(O, Os) -> [O|Os]. diff --git a/lib/compiler/test/Makefile b/lib/compiler/test/Makefile index db8eb7e2e1..7be23fbb93 100644 --- a/lib/compiler/test/Makefile +++ b/lib/compiler/test/Makefile @@ -109,6 +109,8 @@ NO_MOD_OPT = $(NO_OPT) NO_SSA_OPT = $(NO_OPT) +NO_TYPE_OPT = $(NO_OPT) + NO_OPT_MODULES= $(NO_OPT:%=%_no_opt_SUITE) NO_OPT_ERL_FILES= $(NO_OPT_MODULES:%=%.erl) POST_OPT_MODULES= $(NO_OPT:%=%_post_opt_SUITE) @@ -121,6 +123,8 @@ NO_MOD_OPT_MODULES= $(NO_MOD_OPT:%=%_no_module_opt_SUITE) NO_MOD_OPT_ERL_FILES= $(NO_MOD_OPT_MODULES:%=%.erl) NO_SSA_OPT_MODULES= $(NO_SSA_OPT:%=%_no_ssa_opt_SUITE) NO_SSA_OPT_ERL_FILES= $(NO_SSA_OPT_MODULES:%=%.erl) +NO_TYPE_OPT_MODULES= $(NO_TYPE_OPT:%=%_no_type_opt_SUITE) +NO_TYPE_OPT_ERL_FILES= $(NO_TYPE_OPT_MODULES:%=%.erl) ERL_FILES= $(MODULES:%=%.erl) CORE_FILES= $(CORE_MODULES:%=%.core) @@ -150,7 +154,7 @@ EBIN = . # ---------------------------------------------------- make_emakefile: $(NO_OPT_ERL_FILES) $(POST_OPT_ERL_FILES) $(NO_SSA_OPT_ERL_FILES) \ - $(INLINE_ERL_FILES) $(R21_ERL_FILES) $(NO_MOD_OPT_ERL_FILES) + $(INLINE_ERL_FILES) $(R21_ERL_FILES) $(NO_MOD_OPT_ERL_FILES) $(NO_TYPE_OPT_ERL_FILES) $(ERL_TOP)/make/make_emakefile $(ERL_COMPILE_FLAGS) -o$(EBIN) $(MODULES) \ > $(EMAKEFILE) $(ERL_TOP)/make/make_emakefile +no_copt +no_postopt \ @@ -169,6 +173,8 @@ make_emakefile: $(NO_OPT_ERL_FILES) $(POST_OPT_ERL_FILES) $(NO_SSA_OPT_ERL_FILES -o$(EBIN) $(NO_MOD_OPT_MODULES) >> $(EMAKEFILE) $(ERL_TOP)/make/make_emakefile +from_core $(ERL_COMPILE_FLAGS) \ -o$(EBIN) $(CORE_MODULES) >> $(EMAKEFILE) + $(ERL_TOP)/make/make_emakefile +no_type_opt $(ERL_COMPILE_FLAGS) \ + -o$(EBIN) $(NO_TYPE_OPT_MODULES) >> $(EMAKEFILE) tests debug opt: make_emakefile erl $(ERL_MAKE_FLAGS) -make @@ -202,6 +208,10 @@ docs: %_no_module_opt_SUITE.erl: %_SUITE.erl sed -e 's;-module($(basename $<));-module($(basename $@));' $< > $@ +%_no_type_opt_SUITE.erl: %_SUITE.erl + sed -e 's;-module($(basename $<));-module($(basename $@));' $< > $@ + + # ---------------------------------------------------- # Release Target # ---------------------------------------------------- @@ -216,7 +226,8 @@ release_tests_spec: make_emakefile $(INSTALL_DATA) $(NO_OPT_ERL_FILES) $(POST_OPT_ERL_FILES) \ $(INLINE_ERL_FILES) $(R21_ERL_FILES) \ $(NO_MOD_OPT_ERL_FILES) \ - $(NO_SSA_OPT_ERL_FILES) "$(RELSYSDIR)" + $(NO_SSA_OPT_ERL_FILES) \ + $(NO_TYPE_OPT_ERL_FILES) "$(RELSYSDIR)" $(INSTALL_DATA) $(CORE_FILES) "$(RELSYSDIR)" for file in $(ERL_DUMMY_FILES); do \ module=`basename $$file .erl`; \ diff --git a/lib/compiler/test/test_lib.erl b/lib/compiler/test/test_lib.erl index 3348c6e9ea..34410e4b2a 100644 --- a/lib/compiler/test/test_lib.erl +++ b/lib/compiler/test/test_lib.erl @@ -97,7 +97,8 @@ get_data_dir(Config) -> Data2 = re:replace(Data1, "_post_opt_SUITE", "_SUITE", Opts), Data3 = re:replace(Data2, "_inline_SUITE", "_SUITE", Opts), Data4 = re:replace(Data3, "_r21_SUITE", "_SUITE", Opts), - Data = re:replace(Data4, "_no_module_opt_SUITE", "_SUITE", Opts), + Data5 = re:replace(Data4, "_no_module_opt_SUITE", "_SUITE", Opts), + Data = re:replace(Data5, "_no_type_opt_SUITE", "_SUITE", Opts), re:replace(Data, "_no_ssa_opt_SUITE", "_SUITE", Opts). is_cloned_mod(Mod) -> -- cgit v1.2.3