Merge pull request #8824 from jhogberg/john/compiler/refactor-guard-cg

beam_core_to_ssa: Ensure {succeeded,guard} is used in guard context
erlang · Oct 2, 2024 · 6eb6efc · 6eb6efc
2 parents a25dbb8 + a0257d3
commit 6eb6efc
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 7 deletions.
diff --git a/.gitignore b/.gitignore
@@ -224,6 +224,7 @@ JAVADOC-GENERATED
 /lib/compiler/src/beam_opcodes.hrl
 /lib/compiler/src/core_parse.erl
 
+/lib/compiler/test/*_no_bool_opt_SUITE.erl
 /lib/compiler/test/*_no_opt_SUITE.erl
 /lib/compiler/test/*_no_copt_SUITE.erl
 /lib/compiler/test/*_no_copt_ssa_SUITE.erl

diff --git a/lib/compiler/src/beam_core_to_ssa.erl b/lib/compiler/src/beam_core_to_ssa.erl
@@ -2760,6 +2760,10 @@ guard_cg(#cg_seq{arg=Arg,body=Body}, Fail, St0) ->
     {ArgIs,St1} = guard_cg(Arg, Fail, St0),
     {BodyIs,St} = guard_cg(Body, Fail, St1),
     {ArgIs++BodyIs,St};
+guard_cg(#cg_succeeded{set=Set0}, Fail, St0) ->
+    {[#b_set{dst=Dst}=Set],St1} = cg(Set0, St0),
+    {Is,St} = make_succeeded(Dst, {guard, Fail}, St1),
+    {[Set|Is],St};
 guard_cg(G, _Fail, St) ->
     cg(G, St).
 

diff --git a/lib/compiler/test/Makefile b/lib/compiler/test/Makefile
@@ -54,6 +54,10 @@ MODULES= \
 	z_SUITE \
 	test_lib
 
+NO_BOOL_OPT= \
+	guard \
+	andor
+
 NO_OPT= \
 	andor \
 	apply \
@@ -132,6 +136,8 @@ NO_SSA_OPT = $(NO_OPT)
 
 NO_TYPE_OPT = $(NO_OPT)
 
+NO_BOOL_OPT_MODULES= $(NO_BOOL_OPT:%=%_no_bool_opt_SUITE)
+NO_BOOL_OPT_ERL_FILES= $(NO_BOOL_OPT_MODULES:%=%.erl)
 NO_OPT_MODULES= $(NO_OPT:%=%_no_opt_SUITE)
 NO_OPT_ERL_FILES= $(NO_OPT_MODULES:%=%.erl)
 POST_OPT_MODULES= $(NO_OPT:%=%_post_opt_SUITE)
@@ -185,12 +191,16 @@ EBIN = .
 
 DISABLE_SSA_OPT = +no_bool_opt +no_share_opt +no_bsm_opt +no_fun_opt +no_ssa_opt +no_recv_opt
 
-make_emakefile: $(NO_OPT_ERL_FILES) $(POST_OPT_ERL_FILES) $(NO_SSA_OPT_ERL_FILES) \
-  $(NO_CORE_OPT_ERL_FILES) $(NO_CORE_SSA_OPT_ERL_FILES) \
-  $(INLINE_ERL_FILES) $(NO_MOD_OPT_ERL_FILES) $(NO_TYPE_OPT_ERL_FILES) \
-  $(DIALYZER_ERL_FILES) $(COVER_ERL_FILES) $(R25_ERL_FILES)
+make_emakefile: $(NO_BOOL_OPT_ERL_FILES) $(NO_OPT_ERL_FILES) \
+                $(POST_OPT_ERL_FILES) $(NO_SSA_OPT_ERL_FILES) \
+                $(NO_CORE_OPT_ERL_FILES) $(NO_CORE_SSA_OPT_ERL_FILES) \
+                $(INLINE_ERL_FILES) $(NO_MOD_OPT_ERL_FILES) \
+                $(NO_TYPE_OPT_ERL_FILES) $(DIALYZER_ERL_FILES) \
+                $(COVER_ERL_FILES) $(R25_ERL_FILES)
 	$(ERL_TOP)/make/make_emakefile $(ERL_COMPILE_FLAGS) -o$(EBIN) $(MODULES) \
 	  > $(EMAKEFILE)
+	$(ERL_TOP)/make/make_emakefile +no_bool_opt $(ERL_COMPILE_FLAGS) \
+	  -o$(EBIN) $(NO_BOOL_OPT_MODULES) >> $(EMAKEFILE)
 	$(ERL_TOP)/make/make_emakefile +no_copt $(DISABLE_SSA_OPT) +no_postopt \
 	  $(ERL_COMPILE_FLAGS) -o$(EBIN) $(NO_OPT_MODULES) >> $(EMAKEFILE)
 	$(ERL_TOP)/make/make_emakefile $(DISABLE_SSA_OPT) $(ERL_COMPILE_FLAGS) \
@@ -230,6 +240,9 @@ docs:
 # Special targets
 # ----------------------------------------------------
 
+%_no_bool_opt_SUITE.erl: %_SUITE.erl
+	sed -e 's;-module($(basename $<));-module($(basename $@));' $< > $@
+
 %_no_opt_SUITE.erl: %_SUITE.erl
 	sed -e 's;-module($(basename $<));-module($(basename $@));' $< > $@
 
@@ -274,7 +287,10 @@ release_tests_spec: make_emakefile
 	$(INSTALL_DIR) "$(RELSYSDIR)"
 	$(INSTALL_DATA) compiler.spec compiler.cover \
 		$(EMAKEFILE) $(ERL_FILES) "$(RELSYSDIR)"
-	$(INSTALL_DATA) $(NO_OPT_ERL_FILES) $(POST_OPT_ERL_FILES) \
+	$(INSTALL_DATA) \
+		$(NO_BOOL_OPT_ERL_FILES) \
+		$(NO_OPT_ERL_FILES) \
+		$(POST_OPT_ERL_FILES) \
 		$(INLINE_ERL_FILES) \
 	        $(R25_ERL_FILES) \
 		$(NO_CORE_OPT_ERL_FILES) \

diff --git a/lib/compiler/test/guard_SUITE.erl b/lib/compiler/test/guard_SUITE.erl
@@ -41,7 +41,8 @@
 	 check_qlc_hrl/1,andalso_semi/1,t_tuple_size/1,binary_part/1,
 	 bad_constants/1,bad_guards/1,
          guard_in_catch/1,beam_bool_SUITE/1,
-         repeated_type_tests/1,use_after_branch/1]).
+         repeated_type_tests/1,use_after_branch/1,
+         body_in_guard/1]).
 
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
@@ -60,7 +61,7 @@ groups() ->
        basic_andalso_orelse,traverse_dcd,
        check_qlc_hrl,andalso_semi,t_tuple_size,binary_part,
        bad_constants,bad_guards,guard_in_catch,beam_bool_SUITE,
-       repeated_type_tests,use_after_branch]},
+       repeated_type_tests,use_after_branch,body_in_guard]},
      {slow,[],[literal_type_tests,generated_combinations]}].
 
 init_per_suite(Config) ->
@@ -3348,6 +3349,18 @@ use_after_branch_1(A) ->
         false -> {id(Boolean), gaffel}
     end.
 
+%% GH-8733: Benign bug where {succeeded,body} was emitted in a guard context,
+%% crashing the compiler when +no_bool_opt was specified.
+body_in_guard(_Config) ->
+    Pid = self(),
+    Mon = monitor(process, Pid),
+    receive
+        {'DOWN', Mon, process, Pid, _} ->
+            ok
+    after 0 ->
+        demonitor(Mon)
+    end.
+
 %% Call this function to turn off constant propagation.
 id(I) -> I.