Browse Source

cmListFileCache: Enforce proper nesting of flow control statements

Fixes: #19153
Kyle Edwards 5 years ago
parent
commit
12f6e37eb7

+ 118 - 0
Source/cmListFileCache.cxx

@@ -30,6 +30,7 @@ struct cmListFileParser
   bool ParseFunction(const char* name, long line);
   bool ParseFunction(const char* name, long line);
   bool AddArgument(cmListFileLexer_Token* token,
   bool AddArgument(cmListFileLexer_Token* token,
                    cmListFileArgument::Delimiter delim);
                    cmListFileArgument::Delimiter delim);
+  cm::optional<cmListFileContext> CheckNesting();
   cmListFile* ListFile;
   cmListFile* ListFile;
   cmListFileBacktrace Backtrace;
   cmListFileBacktrace Backtrace;
   cmMessenger* Messenger;
   cmMessenger* Messenger;
@@ -158,6 +159,17 @@ bool cmListFileParser::Parse()
       return false;
       return false;
     }
     }
   }
   }
+
+  // Check if all functions are nested properly.
+  if (auto badNesting = this->CheckNesting()) {
+    this->Messenger->IssueMessage(
+      MessageType::FATAL_ERROR,
+      "Flow control statements are not properly nested.",
+      this->Backtrace.Push(*badNesting));
+    cmSystemTools::SetFatalErrorOccured();
+    return false;
+  }
+
   return true;
   return true;
 }
 }
 
 
@@ -317,6 +329,112 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
   return true;
   return true;
 }
 }
 
 
+namespace {
+enum class NestingStateEnum
+{
+  If,
+  Else,
+  While,
+  Foreach,
+  Function,
+  Macro,
+};
+
+struct NestingState
+{
+  NestingStateEnum State;
+  cmListFileContext Context;
+};
+
+bool TopIs(std::vector<NestingState>& stack, NestingStateEnum state)
+{
+  return !stack.empty() && stack.back().State == state;
+}
+}
+
+cm::optional<cmListFileContext> cmListFileParser::CheckNesting()
+{
+  std::vector<NestingState> stack;
+
+  for (auto const& func : this->ListFile->Functions) {
+    auto const& name = func.LowerCaseName();
+    if (name == "if") {
+      stack.push_back({
+        NestingStateEnum::If,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      });
+    } else if (name == "elseif") {
+      if (!TopIs(stack, NestingStateEnum::If)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.back() = {
+        NestingStateEnum::If,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      };
+    } else if (name == "else") {
+      if (!TopIs(stack, NestingStateEnum::If)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.back() = {
+        NestingStateEnum::Else,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      };
+    } else if (name == "endif") {
+      if (!TopIs(stack, NestingStateEnum::If) &&
+          !TopIs(stack, NestingStateEnum::Else)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.pop_back();
+    } else if (name == "while") {
+      stack.push_back({
+        NestingStateEnum::While,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      });
+    } else if (name == "endwhile") {
+      if (!TopIs(stack, NestingStateEnum::While)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.pop_back();
+    } else if (name == "foreach") {
+      stack.push_back({
+        NestingStateEnum::Foreach,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      });
+    } else if (name == "endforeach") {
+      if (!TopIs(stack, NestingStateEnum::Foreach)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.pop_back();
+    } else if (name == "function") {
+      stack.push_back({
+        NestingStateEnum::Function,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      });
+    } else if (name == "endfunction") {
+      if (!TopIs(stack, NestingStateEnum::Function)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.pop_back();
+    } else if (name == "macro") {
+      stack.push_back({
+        NestingStateEnum::Macro,
+        cmListFileContext::FromCommandContext(func, this->FileName),
+      });
+    } else if (name == "endmacro") {
+      if (!TopIs(stack, NestingStateEnum::Macro)) {
+        return cmListFileContext::FromCommandContext(func, this->FileName);
+      }
+      stack.pop_back();
+    }
+  }
+
+  if (!stack.empty()) {
+    return stack.back().Context;
+  }
+
+  return cm::nullopt;
+}
+
 // We hold either the bottom scope of a directory or a call/file context.
 // We hold either the bottom scope of a directory or a call/file context.
 // Discriminate these cases via the parent pointer.
 // Discriminate these cases via the parent pointer.
 struct cmListFileBacktrace::Entry
 struct cmListFileBacktrace::Entry

+ 8 - 0
Source/cmListFileCache.h

@@ -89,6 +89,14 @@ public:
   {
   {
   }
   }
 
 
+#if __cplusplus < 201703L && (!defined(_MSVC_LANG) || _MSVC_LANG < 201703L)
+  cmListFileContext(const cmListFileContext& /*other*/) = default;
+  cmListFileContext(cmListFileContext&& /*other*/) = default;
+
+  cmListFileContext& operator=(const cmListFileContext& /*other*/) = default;
+  cmListFileContext& operator=(cmListFileContext&& /*other*/) = delete;
+#endif
+
   static cmListFileContext FromCommandContext(
   static cmListFileContext FromCommandContext(
     cmCommandContext const& lfcc, std::string const& fileName,
     cmCommandContext const& lfcc, std::string const& fileName,
     cm::optional<std::string> deferId = {})
     cm::optional<std::string> deferId = {})

+ 16 - 44
Tests/CMakeTests/EndStuffTestScript.cmake

@@ -1,68 +1,40 @@
 message(STATUS "testname='${testname}'")
 message(STATUS "testname='${testname}'")
 
 
-if(testname STREQUAL bad_else) # fail
-  file(WRITE "${dir}/${testname}.cmake"
-"else()
-")
+function(do_end content)
+  file(WRITE "${dir}/${testname}.cmake" "${content}")
   execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
   execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
     RESULT_VARIABLE rv)
     RESULT_VARIABLE rv)
   if(NOT rv EQUAL 0)
   if(NOT rv EQUAL 0)
     message(FATAL_ERROR "${testname} failed")
     message(FATAL_ERROR "${testname} failed")
   endif()
   endif()
+endfunction()
+
+if(testname STREQUAL bad_else) # fail
+  do_end("else()\n")
 
 
 elseif(testname STREQUAL bad_elseif) # fail
 elseif(testname STREQUAL bad_elseif) # fail
-  file(WRITE "${dir}/${testname}.cmake"
-"elseif()
-")
-  execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
-    RESULT_VARIABLE rv)
-  if(NOT rv EQUAL 0)
-    message(FATAL_ERROR "${testname} failed")
-  endif()
+  do_end("elseif()\n")
 
 
 elseif(testname STREQUAL bad_endforeach) # fail
 elseif(testname STREQUAL bad_endforeach) # fail
-  endforeach()
+  do_end("endforeach()\n")
 
 
 elseif(testname STREQUAL bad_endfunction) # fail
 elseif(testname STREQUAL bad_endfunction) # fail
-  endfunction()
+  do_end("endfunction()\n")
 
 
 elseif(testname STREQUAL bad_endif) # fail
 elseif(testname STREQUAL bad_endif) # fail
-  file(WRITE "${dir}/${testname}.cmake"
-"cmake_minimum_required(VERSION 2.8)
-endif()
-")
-  execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
-    RESULT_VARIABLE rv)
-  if(NOT rv EQUAL 0)
-    message(FATAL_ERROR "${testname} failed")
-  endif()
+  do_end("cmake_minimum_required(VERSION 2.8)\nendif()\n")
 
 
-elseif(testname STREQUAL endif_low_min_version) # pass
-  file(WRITE "${dir}/${testname}.cmake"
-"cmake_minimum_required(VERSION 1.2)
-endif()
-")
-  execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
-    RESULT_VARIABLE rv)
-  if(NOT rv EQUAL 0)
-    message(FATAL_ERROR "${testname} failed")
-  endif()
+elseif(testname STREQUAL endif_low_min_version) # fail
+  do_end("cmake_minimum_required(VERSION 1.2)\nendif()\n")
 
 
-elseif(testname STREQUAL endif_no_min_version) # pass
-  file(WRITE "${dir}/${testname}.cmake"
-"endif()
-")
-  execute_process(COMMAND ${CMAKE_COMMAND} -P "${dir}/${testname}.cmake"
-    RESULT_VARIABLE rv)
-  if(NOT rv EQUAL 0)
-    message(FATAL_ERROR "${testname} failed")
-  endif()
+elseif(testname STREQUAL endif_no_min_version) # fail
+  do_end("endif()\n")
 
 
 elseif(testname STREQUAL bad_endmacro) # fail
 elseif(testname STREQUAL bad_endmacro) # fail
-  endmacro()
+  do_end("endmacro()\n")
 
 
 elseif(testname STREQUAL bad_endwhile) # fail
 elseif(testname STREQUAL bad_endwhile) # fail
-  endwhile()
+  do_end("endwhile()\n")
 
 
 else() # fail
 else() # fail
   message(FATAL_ERROR "testname='${testname}' - error: no such test in '${CMAKE_CURRENT_LIST_FILE}'")
   message(FATAL_ERROR "testname='${testname}' - error: no such test in '${CMAKE_CURRENT_LIST_FILE}'")

+ 3 - 7
Tests/RunCMake/Syntax/FunctionUnmatched-stderr.txt

@@ -1,8 +1,4 @@
-^CMake Error in FunctionUnmatched.cmake:
-  A logical block opening on the line
-
-    .*/Tests/RunCMake/Syntax/FunctionUnmatched.cmake:[0-9]+ \(function\)
-
-  is not closed.
+^CMake Error at FunctionUnmatched\.cmake:[0-9]+ \(function\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:[0-9]+ \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 3 - 7
Tests/RunCMake/Syntax/FunctionUnmatchedForeach-stderr.txt

@@ -1,8 +1,4 @@
-^CMake Error at FunctionUnmatchedForeach.cmake:[0-9]+ \(f\):
-  A logical block opening on the line
-
-    .*/Tests/RunCMake/Syntax/FunctionUnmatchedForeach.cmake:[0-9]+ \(foreach\)
-
-  is not closed.
+^CMake Error at FunctionUnmatchedForeach\.cmake:[0-9]+ \(endfunction\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:[0-9]+ \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 1 - 0
Tests/RunCMake/Syntax/ImproperNesting-result.txt

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

+ 4 - 0
Tests/RunCMake/Syntax/ImproperNesting-stderr.txt

@@ -0,0 +1,4 @@
+^CMake Error at ImproperNesting\.cmake:[0-9]+ \(endforeach\):
+  Flow control statements are not properly nested\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 7 - 0
Tests/RunCMake/Syntax/ImproperNesting.cmake

@@ -0,0 +1,7 @@
+message(FATAL_ERROR "This should not happen")
+
+foreach(i 1 2)
+  if(1)
+endforeach()
+endif()
+endif()

+ 3 - 7
Tests/RunCMake/Syntax/MacroUnmatched-stderr.txt

@@ -1,8 +1,4 @@
-^CMake Error in MacroUnmatched.cmake:
-  A logical block opening on the line
-
-    .*/Tests/RunCMake/Syntax/MacroUnmatched.cmake:[0-9]+ \(macro\)
-
-  is not closed.
+^CMake Error at MacroUnmatched\.cmake:[0-9]+ \(macro\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:[0-9]+ \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 3 - 7
Tests/RunCMake/Syntax/MacroUnmatchedForeach-stderr.txt

@@ -1,8 +1,4 @@
-^CMake Error at MacroUnmatchedForeach.cmake:[0-9]+ \(m\):
-  A logical block opening on the line
-
-    .*/Tests/RunCMake/Syntax/MacroUnmatchedForeach.cmake:[0-9]+ \(foreach\)
-
-  is not closed.
+^CMake Error at MacroUnmatchedForeach\.cmake:[0-9]+ \(endmacro\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:[0-9]+ \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 1 - 0
Tests/RunCMake/Syntax/RunCMakeTest.cmake

@@ -72,6 +72,7 @@ run_cmake(UnterminatedBrace2)
 run_cmake(UnterminatedBracket0)
 run_cmake(UnterminatedBracket0)
 run_cmake(UnterminatedBracket1)
 run_cmake(UnterminatedBracket1)
 run_cmake(UnterminatedBracketComment)
 run_cmake(UnterminatedBracketComment)
+run_cmake(ImproperNesting)
 
 
 # Variable expansion tests
 # Variable expansion tests
 run_cmake(ExpandInAt)
 run_cmake(ExpandInAt)

+ 3 - 3
Tests/RunCMake/if/duplicate-deep-else-stderr.txt

@@ -1,4 +1,4 @@
-CMake Error at duplicate-deep-else.cmake:[0-9]+ \(else\):
-  A duplicate ELSE command was found inside an IF block.
+CMake Error at duplicate-deep-else\.cmake:[0-9]+ \(else\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)

+ 3 - 3
Tests/RunCMake/if/duplicate-else-after-elseif-stderr.txt

@@ -1,4 +1,4 @@
-CMake Error at duplicate-else-after-elseif.cmake:[0-9]+ \(else\):
-  A duplicate ELSE command was found inside an IF block.
+CMake Error at duplicate-else-after-elseif\.cmake:[0-9]+ \(else\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)

+ 3 - 3
Tests/RunCMake/if/duplicate-else-stderr.txt

@@ -1,4 +1,4 @@
-CMake Error at duplicate-else.cmake:[0-9]+ \(else\):
-  A duplicate ELSE command was found inside an IF block.
+CMake Error at duplicate-else\.cmake:[0-9]+ \(else\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)

+ 3 - 3
Tests/RunCMake/if/misplaced-elseif-stderr.txt

@@ -1,4 +1,4 @@
-CMake Error at misplaced-elseif.cmake:[0-9]+ \(elseif\):
-  An ELSEIF command was found after an ELSE command.
+CMake Error at misplaced-elseif\.cmake:[0-9]+ \(elseif\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
+  CMakeLists\.txt:[0-9]+ \(include\)

+ 3 - 4
Tests/RunCMake/while/EndAlone-stderr.txt

@@ -1,5 +1,4 @@
-^CMake Error at EndAlone.cmake:1 \(endwhile\):
-  endwhile An ENDWHILE command was found outside of a proper WHILE ENDWHILE
-  structure.  Or its arguments did not match the opening WHILE command.
+^CMake Error at EndAlone\.cmake:[0-9]+ \(endwhile\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 3 - 4
Tests/RunCMake/while/EndAloneArgs-stderr.txt

@@ -1,5 +1,4 @@
-^CMake Error at EndAloneArgs.cmake:1 \(endwhile\):
-  endwhile An ENDWHILE command was found outside of a proper WHILE ENDWHILE
-  structure.  Or its arguments did not match the opening WHILE command.
+^CMake Error at EndAloneArgs\.cmake:[0-9]+ \(endwhile\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 3 - 7
Tests/RunCMake/while/EndMissing-stderr.txt

@@ -1,8 +1,4 @@
-^CMake Error in EndMissing.cmake:
-  A logical block opening on the line
-
-    .*/Tests/RunCMake/while/EndMissing.cmake:1 \(while\)
-
-  is not closed.
+^CMake Error at EndMissing\.cmake:[0-9]+ \(while\):
+  Flow control statements are not properly nested\.
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:[0-9]+ \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 9 - 2
Tests/RunCMake/while/MissingArgument-stderr.txt

@@ -1,4 +1,11 @@
-^CMake Error at MissingArgument.cmake:1 \(while\):
+^CMake Error at MissingArgument\.cmake:[0-9]+ \(while\):
   while called with incorrect number of arguments
   while called with incorrect number of arguments
 Call Stack \(most recent call first\):
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)$
+  CMakeLists\.txt:[0-9]+ \(include\)
+
+
+CMake Error at MissingArgument\.cmake:[0-9]+ \(endwhile\):
+  endwhile An ENDWHILE command was found outside of a proper WHILE ENDWHILE
+  structure\.  Or its arguments did not match the opening WHILE command\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)$

+ 1 - 0
Tests/RunCMake/while/MissingArgument.cmake

@@ -1 +1,2 @@
 while()
 while()
+endwhile()