[libunwind] [runtimes] Fix parsing of LIB{CXX,CXXABI,UNWIND}_TEST_PARAMS (PR #67691)

via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 28 08:02:07 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxxabi

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/67691.diff


5 Files Affected:

- (modified) clang/cmake/caches/CrossWinToARMLinux.cmake (+7-3) 
- (modified) libcxx/test/CMakeLists.txt (+3-9) 
- (modified) libcxxabi/test/CMakeLists.txt (+3-5) 
- (modified) libunwind/test/CMakeLists.txt (+3-8) 
- (added) runtimes/cmake/Modules/HandleLitArguments.cmake (+20) 


``````````diff
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()

``````````

</details>


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


More information about the cfe-commits mailing list