cmState: Prohibit override of flow control commands

Source/cmCommands.cxx

@@ -119,12 +119,19 @@
 
 void GetScriptingCommands(cmState* state)
 {
-  state->AddBuiltinCommand("break", cmBreakCommand);
+  state->AddFlowControlCommand("break", cmBreakCommand);
+  state->AddFlowControlCommand("continue", cmContinueCommand);
+  state->AddFlowControlCommand("foreach", cmForEachCommand);
+  state->AddFlowControlCommand("function", cmFunctionCommand);
+  state->AddFlowControlCommand("if", cmIfCommand);
+  state->AddFlowControlCommand("macro", cmMacroCommand);
+  state->AddFlowControlCommand("return", cmReturnCommand);
+  state->AddFlowControlCommand("while", cmWhileCommand);
+
   state->AddBuiltinCommand("cmake_minimum_required", cmCMakeMinimumRequired);
   state->AddBuiltinCommand("cmake_path", cmCMakePathCommand);
   state->AddBuiltinCommand("cmake_policy", cmCMakePolicyCommand);
   state->AddBuiltinCommand("configure_file", cmConfigureFileCommand);
-  state->AddBuiltinCommand("continue", cmContinueCommand);
   state->AddBuiltinCommand("exec_program", cmExecProgramCommand);
   state->AddBuiltinCommand("execute_process", cmExecuteProcessCommand);
   state->AddBuiltinCommand("file", cmFileCommand);
@@ -133,26 +140,21 @@ void GetScriptingCommands(cmState* state)
   state->AddBuiltinCommand("find_package", cmFindPackage);
   state->AddBuiltinCommand("find_path", cmFindPath);
   state->AddBuiltinCommand("find_program", cmFindProgram);
-  state->AddBuiltinCommand("foreach", cmForEachCommand);
-  state->AddBuiltinCommand("function", cmFunctionCommand);
   state->AddBuiltinCommand("get_cmake_property", cmGetCMakePropertyCommand);
   state->AddBuiltinCommand("get_directory_property",
                            cmGetDirectoryPropertyCommand);
   state->AddBuiltinCommand("get_filename_component",
                            cmGetFilenameComponentCommand);
   state->AddBuiltinCommand("get_property", cmGetPropertyCommand);
-  state->AddBuiltinCommand("if", cmIfCommand);
   state->AddBuiltinCommand("include", cmIncludeCommand);
   state->AddBuiltinCommand("include_guard", cmIncludeGuardCommand);
   state->AddBuiltinCommand("list", cmListCommand);
-  state->AddBuiltinCommand("macro", cmMacroCommand);
   state->AddBuiltinCommand("make_directory", cmMakeDirectoryCommand);
   state->AddBuiltinCommand("mark_as_advanced", cmMarkAsAdvancedCommand);
   state->AddBuiltinCommand("math", cmMathCommand);
   state->AddBuiltinCommand("message", cmMessageCommand);
   state->AddBuiltinCommand("option", cmOptionCommand);
   state->AddBuiltinCommand("cmake_parse_arguments", cmParseArgumentsCommand);
-  state->AddBuiltinCommand("return", cmReturnCommand);
   state->AddBuiltinCommand("separate_arguments", cmSeparateArgumentsCommand);
   state->AddBuiltinCommand("set", cmSetCommand);
   state->AddBuiltinCommand("set_directory_properties",
@@ -161,7 +163,6 @@ void GetScriptingCommands(cmState* state)
   state->AddBuiltinCommand("site_name", cmSiteNameCommand);
   state->AddBuiltinCommand("string", cmStringCommand);
   state->AddBuiltinCommand("unset", cmUnsetCommand);
-  state->AddBuiltinCommand("while", cmWhileCommand);
 
   state->AddUnexpectedCommand(
     "else",

Source/cmFunctionCommand.cxx

@@ -163,8 +163,11 @@ bool cmFunctionFunctionBlocker::Replay(
   f.FilePath = this->GetStartingContext().FilePath;
   f.Line = this->GetStartingContext().Line;
   mf.RecordPolicies(f.Policies);
-  mf.GetState()->AddScriptedCommand(this->Args.front(), std::move(f));
-  return true;
+  return mf.GetState()->AddScriptedCommand(
+    this->Args.front(),
+    BT<cmState::Command>(std::move(f),
+                         mf.GetBacktrace().Push(this->GetStartingContext())),
+    mf);
 }
 
 } // anonymous namespace

Source/cmLoadCommandCommand.cxx

@@ -24,6 +24,7 @@
 #include "cmCommand.h"
 #include "cmDynamicLoader.h"
 #include "cmExecutionStatus.h"
+#include "cmListFileCache.h"
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
 #include "cmState.h"
@@ -36,8 +37,6 @@
 #  include <malloc.h> /* for malloc/free on QNX */
 #endif
 
-class cmListFileBacktrace;
-
 namespace {
 
 const char* LastName = nullptr;
@@ -256,10 +255,12 @@ bool cmLoadCommandCommand(std::vector<std::string> const& args,
   // if the symbol is found call it to set the name on the
   // function blocker
   if (initFunction) {
-    status.GetMakefile().GetState()->AddScriptedCommand(
+    return status.GetMakefile().GetState()->AddScriptedCommand(
       args[0],
-      cmLegacyCommandWrapper(cm::make_unique<cmLoadedCommand>(initFunction)));
-    return true;
+      BT<cmState::Command>(
+        cmLegacyCommandWrapper(cm::make_unique<cmLoadedCommand>(initFunction)),
+        status.GetMakefile().GetBacktrace()),
+      status.GetMakefile());
   }
   status.SetError("Attempt to load command failed. "
                   "No init function found.");

Source/cmMacroCommand.cxx

@@ -171,8 +171,11 @@ bool cmMacroFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
   f.Functions = std::move(functions);
   f.FilePath = this->GetStartingContext().FilePath;
   mf.RecordPolicies(f.Policies);
-  mf.GetState()->AddScriptedCommand(this->Args[0], std::move(f));
-  return true;
+  return mf.GetState()->AddScriptedCommand(
+    this->Args[0],
+    BT<cmState::Command>(std::move(f),
+                         mf.GetBacktrace().Push(this->GetStartingContext())),
+    mf);
 }
 }
 

Source/cmState.cxx

@@ -436,6 +436,19 @@ void cmState::AddBuiltinCommand(std::string const& name,
     });
 }
 
+void cmState::AddFlowControlCommand(std::string const& name, Command command)
+{
+  this->FlowControlCommands.insert(name);
+  this->AddBuiltinCommand(name, std::move(command));
+}
+
+void cmState::AddFlowControlCommand(std::string const& name,
+                                    BuiltinCommand command)
+{
+  this->FlowControlCommands.insert(name);
+  this->AddBuiltinCommand(name, command);
+}
+
 void cmState::AddDisallowedCommand(std::string const& name,
                                    BuiltinCommand command,
                                    cmPolicies::PolicyID policy,
@@ -465,7 +478,7 @@ void cmState::AddDisallowedCommand(std::string const& name,
 
 void cmState::AddUnexpectedCommand(std::string const& name, const char* error)
 {
-  this->AddBuiltinCommand(
+  this->AddFlowControlCommand(
     name,
     [name, error](std::vector<cmListFileArgument> const&,
                   cmExecutionStatus& status) -> bool {
@@ -480,16 +493,28 @@ void cmState::AddUnexpectedCommand(std::string const& name, const char* error)
     });
 }
 
-void cmState::AddScriptedCommand(std::string const& name, Command command)
+bool cmState::AddScriptedCommand(std::string const& name, BT<Command> command,
+                                 cmMakefile& mf)
 {
   std::string sName = cmSystemTools::LowerCase(name);
 
+  if (this->FlowControlCommands.count(sName)) {
+    mf.GetCMakeInstance()->IssueMessage(
+      MessageType::FATAL_ERROR,
+      cmStrCat("Built-in flow control command \"", sName,
+               "\" cannot be overridden."),
+      command.Backtrace);
+    cmSystemTools::SetFatalErrorOccured();
+    return false;
+  }
+
   // if the command already exists, give a new name to the old command.
   if (Command oldCmd = this->GetCommandByExactName(sName)) {
     this->ScriptedCommands["_" + sName] = oldCmd;
   }
 
-  this->ScriptedCommands[sName] = std::move(command);
+  this->ScriptedCommands[sName] = std::move(command.Value);
+  return true;
 }
 
 cmState::Command cmState::GetCommand(std::string const& name) const

Source/cmState.h

@@ -9,6 +9,7 @@
 #include <set>
 #include <string>
 #include <unordered_map>
+#include <unordered_set>
 #include <vector>
 
 #include "cmDefinitions.h"
@@ -24,6 +25,7 @@
 class cmCacheManager;
 class cmCommand;
 class cmGlobVerificationManager;
+class cmMakefile;
 class cmStateSnapshot;
 class cmMessenger;
 class cmExecutionStatus;
@@ -157,10 +159,13 @@ public:
                          std::unique_ptr<cmCommand> command);
   void AddBuiltinCommand(std::string const& name, Command command);
   void AddBuiltinCommand(std::string const& name, BuiltinCommand command);
+  void AddFlowControlCommand(std::string const& name, Command command);
+  void AddFlowControlCommand(std::string const& name, BuiltinCommand command);
   void AddDisallowedCommand(std::string const& name, BuiltinCommand command,
                             cmPolicies::PolicyID policy, const char* message);
   void AddUnexpectedCommand(std::string const& name, const char* error);
-  void AddScriptedCommand(std::string const& name, Command command);
+  bool AddScriptedCommand(std::string const& name, BT<Command> command,
+                          cmMakefile& mf);
   void RemoveBuiltinCommand(std::string const& name);
   void RemoveUserDefinedCommands();
   std::vector<std::string> GetCommandNames() const;
@@ -223,6 +228,7 @@ private:
   std::vector<std::string> EnabledLanguages;
   std::unordered_map<std::string, Command> BuiltinCommands;
   std::unordered_map<std::string, Command> ScriptedCommands;
+  std::unordered_set<std::string> FlowControlCommands;
   cmPropertyMap GlobalProperties;
   std::unique_ptr<cmCacheManager> CacheManager;
   std::unique_ptr<cmGlobVerificationManager> GlobVerificationManager;

Tests/RunCMake/Syntax/Override.cmake

@@ -0,0 +1,6 @@
+function(override)
+  function(${FUNCTION_NAME})
+  endfunction()
+endfunction()
+override()
+message(FATAL_ERROR "This shouldn't happen")

Tests/RunCMake/Syntax/OverrideBreak-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideContinue-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideElse-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideElseIf-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideEndForeach-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideEndFunction-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideEndIf-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideEndMacro-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideEndWhile-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideForeach-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideFunction-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideIf-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideMacro-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideReturn-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/OverrideWhile-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/Syntax/RunCMakeTest.cmake

@@ -122,3 +122,30 @@ run_cmake(FunctionUnmatched)
 run_cmake(FunctionUnmatchedForeach)
 run_cmake(MacroUnmatched)
 run_cmake(MacroUnmatchedForeach)
+
+function(run_override name)
+  string(TOLOWER "${name}" lname)
+  set(RunCMake_DEFAULT_stderr "^CMake Error at [^
+]*/Tests/RunCMake/Syntax/Override\\.cmake:[0-9]+ \\(function\\):
+  Built-in flow control command \"${lname}\" cannot be overridden\\.
+Call Stack \\(most recent call first\\):
+  [^
+]*/Tests/RunCMake/Syntax/Override\\.cmake:[0-9]+ \\(override\\)$")
+  run_cmake_command(Override${name} "${CMAKE_COMMAND}" -DFUNCTION_NAME=${name} -P "${RunCMake_SOURCE_DIR}/Override.cmake")
+endfunction()
+
+run_override(Break)
+run_override(Continue)
+run_override(Else)
+run_override(ElseIf)
+run_override(EndForeach)
+run_override(EndFunction)
+run_override(EndIf)
+run_override(EndMacro)
+run_override(EndWhile)
+run_override(Foreach)
+run_override(Function)
+run_override(If)
+run_override(Macro)
+run_override(Return)
+run_override(While)