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

Alexander Richardson via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 29 15:17:13 PDT 2023


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

>From 70cfa2f35c152528e6c6e05690ce079c71169fc7 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Fri, 29 Sep 2023 09:03:01 -0700
Subject: [PATCH 1/2] [runtimes] Factor out serialize_lit_param to a separate
 file

This introduces new macros that expect an output variable and a new
macro serialize_lit_string_param() that ensure that the serialized
value is escaped appropriately to store it into a python change.
---
 libcxx/test/CMakeLists.txt                    | 27 +++++++++----------
 libcxxabi/test/CMakeLists.txt                 | 21 +++++++--------
 libunwind/test/CMakeLists.txt                 | 17 +++++-------
 .../cmake/Modules/HandleLitArguments.cmake    | 10 +++++++
 4 files changed, 38 insertions(+), 37 deletions(-)
 create mode 100644 runtimes/cmake/Modules/HandleLitArguments.cmake

diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index 0d5fa6959f17881..b06b9cb8616b30e 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -1,3 +1,4 @@
+include(HandleLitArguments)
 add_subdirectory(tools)
 
 # By default, libcxx and libcxxabi share a library directory.
@@ -9,39 +10,35 @@ 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}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS executor "${LIBCXX_EXECUTOR}")
 endif()
 
 if (NOT LIBCXX_ENABLE_EXCEPTIONS)
-  serialize_lit_param(enable_exceptions False)
+  serialize_lit_param(SERIALIZED_LIT_PARAMS enable_exceptions False)
 endif()
 
 if (NOT LIBCXX_ENABLE_RTTI)
-  serialize_lit_param(enable_rtti False)
+  serialize_lit_param(SERIALIZED_LIT_PARAMS enable_rtti False)
 endif()
 
-serialize_lit_param(hardening_mode "\"${LIBCXX_HARDENING_MODE}\"")
+serialize_lit_string_param(SERIALIZED_LIT_PARAMS hardening_mode "${LIBCXX_HARDENING_MODE}")
 
 if (CMAKE_CXX_COMPILER_TARGET)
-  serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS target_triple "${CMAKE_CXX_COMPILER_TARGET}")
 else()
-  serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS target_triple "${LLVM_DEFAULT_TARGET_TRIPLE}")
 endif()
 
 if (LLVM_USE_SANITIZER)
-  serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
 endforeach()
 
 if (NOT DEFINED LIBCXX_TEST_DEPS)
@@ -68,9 +65,9 @@ if (MSVC)
     set(cxx_lib "${cxx_lib}d")
   endif()
 
-  serialize_lit_param(dbg_include "\"${dbg_include}\"")
-  serialize_lit_param(fms_runtime_lib "\"${fms_runtime_lib}\"")
-  serialize_lit_param(cxx_lib "\"${cxx_lib}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS dbg_include "${dbg_include}")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS fms_runtime_lib "${fms_runtime_lib}")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS cxx_lib "${cxx_lib}")
 endif()
 
 if (LIBCXX_INCLUDE_TESTS)
diff --git a/libcxxabi/test/CMakeLists.txt b/libcxxabi/test/CMakeLists.txt
index b65ac3a9d16422c..0453a82d9db3e37 100644
--- a/libcxxabi/test/CMakeLists.txt
+++ b/libcxxabi/test/CMakeLists.txt
@@ -1,4 +1,5 @@
 include(AddLLVM) # for configure_lit_site_cfg and add_lit_testsuite
+include(HandleLitArguments)
 macro(pythonize_bool var)
   if (${var})
     set(${var} True)
@@ -23,39 +24,35 @@ endif()
 set(AUTO_GEN_COMMENT "## Autogenerated by libcxxabi 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 (LIBCXXABI_EXECUTOR)
   message(DEPRECATION "LIBCXXABI_EXECUTOR is deprecated, please add executor=... to LIBCXXABI_TEST_PARAMS")
-  serialize_lit_param(executor "\"${LIBCXXABI_EXECUTOR}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS executor "${LIBCXXABI_EXECUTOR}")
 endif()
 
 if (NOT LIBCXXABI_ENABLE_EXCEPTIONS)
-  serialize_lit_param(enable_exceptions False)
+  serialize_lit_param(SERIALIZED_LIT_PARAMS enable_exceptions False)
 endif()
 
 if (LIBCXXABI_ENABLE_ASSERTIONS)
-  serialize_lit_param(enable_assertions True)
+  serialize_lit_param(SERIALIZED_LIT_PARAMS enable_assertions True)
 endif()
 
-serialize_lit_param(enable_experimental False)
+serialize_lit_param(SERIALIZED_LIT_PARAMS enable_experimental False)
 
 if (LLVM_USE_SANITIZER)
-  serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS use_sanitizer "${LLVM_USE_SANITIZER}")
 endif()
 
 if (CMAKE_CXX_COMPILER_TARGET)
-  serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS target_triple "${CMAKE_CXX_COMPILER_TARGET}")
 else()
-  serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
 endforeach()
 
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/configs/cmake-bridge.cfg.in"
diff --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index 084da0ab8f354d7..3493a7f49837377 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -1,4 +1,5 @@
 include(AddLLVM) # for add_lit_testsuite
+include(HandleLitArguments)
 macro(pythonize_bool var)
   if (${var})
     set(${var} True)
@@ -14,31 +15,27 @@ 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")
-  serialize_lit_param(executor "\"${LIBUNWIND_EXECUTOR}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS executor "${LIBUNWIND_EXECUTOR}")
 endif()
 
-serialize_lit_param(enable_experimental False)
+serialize_lit_param(SERIALIZED_LIT_PARAMS enable_experimental False)
 
 if (LLVM_USE_SANITIZER)
-  serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS use_sanitizer "${LLVM_USE_SANITIZER}")
 endif()
 
 if (CMAKE_CXX_COMPILER_TARGET)
-  serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS target_triple "${CMAKE_CXX_COMPILER_TARGET}")
 else()
-  serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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}\"")
+  serialize_lit_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
 endforeach()
 
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/configs/cmake-bridge.cfg.in"
diff --git a/runtimes/cmake/Modules/HandleLitArguments.cmake b/runtimes/cmake/Modules/HandleLitArguments.cmake
new file mode 100644
index 000000000000000..e7401c02dd2ec66
--- /dev/null
+++ b/runtimes/cmake/Modules/HandleLitArguments.cmake
@@ -0,0 +1,10 @@
+
+macro(serialize_lit_param output_var param value)
+  string(APPEND ${output_var} "config.${param} = ${value}\n")
+endmacro()
+
+macro(serialize_lit_string_param output_var param value)
+  # Ensure that all quotes in the value are escaped for a valid python string.
+  string(REPLACE "\"" "\\\"" _escaped_value "${value}")
+  string(APPEND ${output_var} "config.${param} = \"${_escaped_value}\"\n")
+endmacro()

>From b4e14f87d4c21cad3cb58502dc3348ba4af36822 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Fri, 29 Sep 2023 09:07:15 -0700
Subject: [PATCH 2/2] [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     | 14 +++++++++++---
 libcxx/test/CMakeLists.txt                      |  6 +-----
 libcxxabi/test/CMakeLists.txt                   |  6 +-----
 libunwind/test/CMakeLists.txt                   |  6 +-----
 runtimes/cmake/Modules/HandleLitArguments.cmake | 10 ++++++++++
 5 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/clang/cmake/caches/CrossWinToARMLinux.cmake b/clang/cmake/caches/CrossWinToARMLinux.cmake
index fd341b182fd6563..2a0953af53faddf 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_default "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS_default "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS_default "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_TEST_PARAMS}")
+
 # Remote test configuration.
 if(DEFINED REMOTE_TEST_HOST)
   # Allow override with the custom values.
@@ -162,11 +166,15 @@ 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_default "executor=${DEFAULT_TEST_EXECUTOR}")
+  list(APPEND RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS_default "executor=${DEFAULT_TEST_EXECUTOR}")
+  list(APPEND RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS_default    "executor=${DEFAULT_TEST_EXECUTOR}")
 endif()
 
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBUNWIND_TEST_PARAMS_default}" CACHE INTERNAL "")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXXABI_TEST_PARAMS_default}" CACHE INTERNAL "")
+set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS "${RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_LIBCXX_TEST_PARAMS_default}" CACHE INTERNAL "")
+
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index b06b9cb8616b30e..48dd233462ab3b7 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -35,11 +35,7 @@ if (LLVM_USE_SANITIZER)
   serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
-endforeach()
+serialize_lit_params_list(SERIALIZED_LIT_PARAMS 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 0453a82d9db3e37..9f95c736d63f464 100644
--- a/libcxxabi/test/CMakeLists.txt
+++ b/libcxxabi/test/CMakeLists.txt
@@ -49,11 +49,7 @@ else()
   serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
-endforeach()
+serialize_lit_params_list(SERIALIZED_LIT_PARAMS 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 3493a7f49837377..21dfbb0a84f0a80 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -32,11 +32,7 @@ else()
   serialize_lit_string_param(SERIALIZED_LIT_PARAMS 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_string_param(SERIALIZED_LIT_PARAMS "${name}" "${value}")
-endforeach()
+serialize_lit_params_list(SERIALIZED_LIT_PARAMS 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
index e7401c02dd2ec66..ccd5dcce4f5bc7e 100644
--- a/runtimes/cmake/Modules/HandleLitArguments.cmake
+++ b/runtimes/cmake/Modules/HandleLitArguments.cmake
@@ -8,3 +8,13 @@ macro(serialize_lit_string_param output_var param value)
   string(REPLACE "\"" "\\\"" _escaped_value "${value}")
   string(APPEND ${output_var} "config.${param} = \"${_escaped_value}\"\n")
 endmacro()
+
+macro(serialize_lit_params_list output_var 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("${output_var}" "${name}" "${value}")
+  endforeach()
+endmacro()



More information about the cfe-commits mailing list