https://github.com/arichardson created 
https://github.com/llvm/llvm-project/pull/67691

Since 78d649a417b48cb8a2ba2e755f0e7c8fb8b1bb83 the recommended way to pass an 
executor is to use the _TEST_PARAMS variable, which means we now pass more 
complicated value (including ones that may contain multiple `=`) as part of 
this variable. However, the `REGEX REPLACE` being used has greedy matches so 
everything up to the last = becomes part of the variable name which results in 
invalid syntax in the generated lit config file.

This was noticed due to builder failures for those using the 
CrossWinToARMLinux.cmake cache file.

>From 7f2045e7f8bf1ef96e5550b64786c25872f61efe Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichard...@google.com>
Date: Thu, 28 Sep 2023 07:21:35 -0700
Subject: [PATCH] [runtimes] Fix parsing of LIB{CXX,CXXABI,UNWIND}_TEST_PARAMS

Since 78d649a417b48cb8a2ba2e755f0e7c8fb8b1bb83 the recommended way to pass
an executor is to use the _TEST_PARAMS variable, which means we now pass
more complicated value (including ones that may contain multiple `=`) as
part of this variable. However, the `REGEX REPLACE` being used has greedy
matches so everything up to the last = becomes part of the variable name
which results in invalid syntax in the generated lit config file.

This was noticed due to builder failures for those using the
CrossWinToARMLinux.cmake cache file.

Co-authored-by: Vladimir Vereschaka <vveresch...@accesssoftek.com>
---
 clang/cmake/caches/CrossWinToARMLinux.cmake   | 10 +++++++---
 libcxx/test/CMakeLists.txt                    | 12 +++--------
 libcxxabi/test/CMakeLists.txt                 |  8 +++-----
 libunwind/test/CMakeLists.txt                 | 11 +++-------
 .../cmake/Modules/HandleLitArguments.cmake    | 20 +++++++++++++++++++
 5 files changed, 36 insertions(+), 25 deletions(-)
 create mode 100644 runtimes/cmake/Modules/HandleLitArguments.cmake

diff --git a/clang/cmake/caches/CrossWinToARMLinux.cmake 
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index fd341b182fd6563..a4dba4d8666027b 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -151,6 +151,10 @@ 
set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
 
 find_package(Python3 COMPONENTS Interpreter)
 
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS 
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}" CACHE INTERNAL "")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS 
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}" CACHE INTERNAL "")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS 
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}" CACHE INTERNAL "")
+
 # Remote test configuration.
 if(DEFINED REMOTE_TEST_HOST)
   # Allow override with the custom values.
@@ -162,9 +166,9 @@ if(DEFINED REMOTE_TEST_HOST)
         "\\\"${Python3_EXECUTABLE}\\\" 
\\\"${LLVM_PROJECT_DIR}/llvm/utils/remote-exec.py\\\" 
--host=${REMOTE_TEST_USER}@${REMOTE_TEST_HOST}"
         CACHE STRING "")
 
-  set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS 
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS} 
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
-  set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS 
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS} 
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
-  set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS    
"${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS} 
'executor=${DEFAULT_TEST_EXECUTOR}'" CACHE STRING "")
+  list(APPEND RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS 
"executor=${DEFAULT_TEST_EXECUTOR}")
+  list(APPEND RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS 
"executor=${DEFAULT_TEST_EXECUTOR}")
+  list(APPEND RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS    
"executor=${DEFAULT_TEST_EXECUTOR}")
 endif()
 
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index 0d5fa6959f17881..0d4771efaf725f4 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -1,3 +1,5 @@
+include(HandleLitArguments)
+
 add_subdirectory(tools)
 
 # By default, libcxx and libcxxabi share a library directory.
@@ -9,10 +11,6 @@ endif()
 set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not 
edit!")
 set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to 
pick them up\n")
 
-macro(serialize_lit_param param value)
-  string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = ${value}\n")
-endmacro()
-
 if (LIBCXX_EXECUTOR)
   message(DEPRECATION "LIBCXX_EXECUTOR is deprecated, please add executor=... 
to LIBCXX_TEST_PARAMS")
   serialize_lit_param(executor "\"${LIBCXX_EXECUTOR}\"")
@@ -38,11 +36,7 @@ if (LLVM_USE_SANITIZER)
   serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
 endif()
 
-foreach(param IN LISTS LIBCXX_TEST_PARAMS)
-  string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")
-  string(REGEX REPLACE "(.+)=(.+)" "\\2" value "${param}")
-  serialize_lit_param("${name}" "\"${value}\"")
-endforeach()
+serialize_lit_params_list(LIBCXX_TEST_PARAMS)
 
 if (NOT DEFINED LIBCXX_TEST_DEPS)
   message(FATAL_ERROR "Expected LIBCXX_TEST_DEPS to be defined")
diff --git a/libcxxabi/test/CMakeLists.txt b/libcxxabi/test/CMakeLists.txt
index b65ac3a9d16422c..bc601dd44058d12 100644
--- a/libcxxabi/test/CMakeLists.txt
+++ b/libcxxabi/test/CMakeLists.txt
@@ -1,4 +1,6 @@
 include(AddLLVM) # for configure_lit_site_cfg and add_lit_testsuite
+include(HandleLitArguments)
+
 macro(pythonize_bool var)
   if (${var})
     set(${var} True)
@@ -52,11 +54,7 @@ else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
-foreach(param IN LISTS LIBCXXABI_TEST_PARAMS)
-  string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")
-  string(REGEX REPLACE "(.+)=(.+)" "\\2" value "${param}")
-  serialize_lit_param("${name}" "\"${value}\"")
-endforeach()
+serialize_lit_params_list(LIBCXXABI_TEST_PARAMS)
 
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/configs/cmake-bridge.cfg.in"
                "${CMAKE_CURRENT_BINARY_DIR}/cmake-bridge.cfg"
diff --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index 084da0ab8f354d7..2e99c182862ae1f 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -1,4 +1,6 @@
 include(AddLLVM) # for add_lit_testsuite
+include(HandleLitArguments)
+
 macro(pythonize_bool var)
   if (${var})
     set(${var} True)
@@ -14,9 +16,6 @@ pythonize_bool(LIBUNWIND_USES_ARM_EHABI)
 set(AUTO_GEN_COMMENT "## Autogenerated by libunwind configuration.\n# Do not 
edit!")
 set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to 
pick them up\n")
 
-macro(serialize_lit_param param value)
-  string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = ${value}\n")
-endmacro()
 
 if (LIBUNWIND_EXECUTOR)
   message(DEPRECATION "LIBUNWIND_EXECUTOR is deprecated, please add 
executor=... to LIBUNWIND_TEST_PARAMS")
@@ -35,11 +34,7 @@ else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
-foreach(param IN LISTS LIBUNWIND_TEST_PARAMS)
-  string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")
-  string(REGEX REPLACE "(.+)=(.+)" "\\2" value "${param}")
-  serialize_lit_param("${name}" "\"${value}\"")
-endforeach()
+serialize_lit_params_list(LIBUNWIND_TEST_PARAMS)
 
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/configs/cmake-bridge.cfg.in"
                "${CMAKE_CURRENT_BINARY_DIR}/cmake-bridge.cfg"
diff --git a/runtimes/cmake/Modules/HandleLitArguments.cmake 
b/runtimes/cmake/Modules/HandleLitArguments.cmake
new file mode 100644
index 000000000000000..00bbe931c3fcdbf
--- /dev/null
+++ b/runtimes/cmake/Modules/HandleLitArguments.cmake
@@ -0,0 +1,20 @@
+
+macro(serialize_lit_param param value)
+  string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = ${value}\n")
+endmacro()
+
+macro(serialize_lit_string_param param value)
+  # Ensure that all quotes in the value are escaped for a valid python string.
+  string(REPLACE "\"" "\\\"" _escaped_value "${value}")
+  string(APPEND SERIALIZED_LIT_PARAMS "config.${param} = 
\"${_escaped_value}\"\n")
+endmacro()
+
+macro(serialize_lit_params_list list)
+  foreach(param IN LISTS ${list})
+    string(FIND "${param}" "=" _eq_index)
+    string(SUBSTRING "${param}" 0 ${_eq_index} name)
+    string(SUBSTRING "${param}" ${_eq_index} -1 value)
+    string(SUBSTRING "${value}" 1 -1 value) # strip the leading =
+    serialize_lit_string_param("${name}" "${value}")
+  endforeach()
+endmacro()

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to