[clang] [runtimes] Fix parsing of LIB{CXX, CXXABI, UNWIND}_TEST_PARAMS (PR #67691)
Alexander Richardson via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 28 08:00:52 PDT 2023
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 <alexrichardson at 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 <vvereschaka at 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()
More information about the cfe-commits
mailing list