Add syntax check of relup to check_install_release and install_release

This commit adds a check that all instructions in the relup are
valid. Earlier, the last part (after point_of_no_return) was not
checked before the actual installation started, meaning that if the
relup was hand written, there was a potential for crashing the upgrade
here.

2 files changed, 85 insertions, 20 deletions

diff --git a/lib/sasl/src/release_handler_1.erl b/lib/sasl/src/release_handler_1.erl
index 3a64e10185..b4b288646f 100644
--- a/lib/sasl/src/release_handler_1.erl
+++ b/lib/sasl/src/release_handler_1.erl
@@ -60,21 +60,25 @@ check_script(Script, LibDirs) ->
     end.
 
 do_check_script(Script, LibDirs, PurgeMods) ->
-    {Before, _After} = split_instructions(Script),
+    {Before, After} = split_instructions(Script),
     case catch lists:foldl(fun(Instruction, EvalState1) ->
 				   eval(Instruction, EvalState1)
 			   end,
 			   #eval_state{libdirs = LibDirs},
 			   Before) of
 	       EvalState2 when is_record(EvalState2, eval_state) ->
-		 {ok,PurgeMods};
+		 case catch syntax_check_script(After) of
+		     ok ->
+			 {ok,PurgeMods};
+		     Other ->
+			 {error,Other}
+		 end;
 	       {error, Error} ->
 		 {error, Error};
 	       Other ->
 		 {error, Other}
 	 end.
 
-
 %% eval_script/1 - For testing only - no apps added, just testing instructions
 eval_script(Script) ->
     eval_script(Script, [], [], [], []).
@@ -91,22 +95,30 @@ eval_script(Script, Apps, LibDirs, NewLibs, Opts) ->
 					       newlibs = NewLibs,
 					       opts = Opts},
 				   Before) of
-		EvalState2 when is_record(EvalState2, eval_state) ->
-		    case catch lists:foldl(fun(Instruction, EvalState3) ->
-						   eval(Instruction, EvalState3)
-					   end,
-					   EvalState2,
-					   After) of
-			EvalState4 when is_record(EvalState4, eval_state) ->
-			    {ok, EvalState4#eval_state.unpurged};
-			restart_emulator ->
-			    restart_emulator;
-			Error ->
-			    {'EXIT', Error}
-		    end;
-		{error, Error} -> {error, Error};
-		Other -> {error, Other}
-	    end;
+		       EvalState2 when is_record(EvalState2, eval_state) ->
+			 case catch syntax_check_script(After) of
+			     ok ->
+				 case catch lists:foldl(
+					      fun(Instruction, EvalState3) ->
+						      eval(Instruction,
+							   EvalState3)
+					      end,
+					      EvalState2,
+					      After) of
+				     EvalState4
+				       when is_record(EvalState4, eval_state) ->
+					 {ok, EvalState4#eval_state.unpurged};
+				     restart_emulator ->
+					 restart_emulator;
+				     Error ->
+					 {'EXIT', Error}
+				 end;
+			     Other ->
+				 {error,Other}
+			 end;
+		       {error, Error} -> {error, Error};
+		       Other -> {error, Other}
+		 end;
 	{error, Mod} ->
 	    {error, {old_processes, Mod}}
     end.
@@ -182,6 +194,42 @@ do_check_old_code(Mod,Procs) ->
     [Mod].
 
 
+%% Check the last part of the script, i.e. the part after point_of_no_return.
+%% More or less a syntax check in case the script was handwritten.
+syntax_check_script([point_of_no_return | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{load, {_,_,_}} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{remove, {_,_,_}} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{purge, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{suspend, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{resume, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{code_change, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{code_change, _, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{stop, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{start, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{sync_nodes, _, {_,_,_}} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{sync_nodes, _, _} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([{apply, {_,_,_}} | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([restart_emulator | Script]) ->
+    syntax_check_script(Script);
+syntax_check_script([Illegal | _Script]) ->
+    throw({illegal_instruction_after_point_of_no_return,Illegal});
+syntax_check_script([]) ->
+    ok.
+
+
 %%-----------------------------------------------------------------
 %% An unpurged module is a module for which there exist an old
 %% version of the code.  This should only be the case if there are
diff --git a/lib/sasl/test/release_handler_SUITE.erl b/lib/sasl/test/release_handler_SUITE.erl
index 958f6bbfc9..12f86365a3 100644
--- a/lib/sasl/test/release_handler_SUITE.erl
+++ b/lib/sasl/test/release_handler_SUITE.erl
@@ -60,7 +60,8 @@ cases() ->
     [otp_2740, otp_2760, otp_5761, otp_9402, otp_9417,
      otp_9395_check_old_code, otp_9395_check_and_purge,
      otp_9395_update_many_mods, otp_9395_rm_many_mods,
-     instructions, eval_appup, supervisor_which_children_timeout].
+     instructions, eval_appup, supervisor_which_children_timeout,
+     install_release_syntax_check].
 
 groups() ->
     [{release,[],
@@ -620,6 +621,22 @@ supervisor_which_children_timeout(Conf) ->
 supervisor_which_children_timeout(cleanup, Conf) ->
     stop_node(node_name(supervisor_which_children_timeout)).
 
+
+%% Test that check_install_release will fail for illegal relup
+%% instructions, even after point of no return.
+install_release_syntax_check(Conf) when is_list(Conf) ->
+
+    S1 = [point_of_no_return, illegal_instruction],
+    {error,{illegal_instruction_after_point_of_no_return,illegal_instruction}} =
+	release_handler_1:check_script(S1,[]),
+
+    S2 = [point_of_no_return,restart_new_emulator],
+    {error,{illegal_instruction_after_point_of_no_return,restart_new_emulator}} =
+	release_handler_1:check_script(S2,[]),
+
+    ok.
+
+
 %%-----------------------------------------------------------------
 %% Ticket: OTP-2740
 %% Slogan: vsn not numeric doesn't work so good in release_handling