Move LIBC_CONF_STRING_UNSAFE_WIDE_READ to top-level libc-configuration #165046
Conversation
This options sets a compile option when building sources inside the string directory, and this option affects string_utils.h. But string_utils.h is #included from more places than just the string directory (such as from __support/CPP/string.h), leading to both narrow-reads in those cases, but more seriously, ODR violations when the two different string_length implementations are included int he same program. Having this option at the top level avoids this problem.
|
@llvm/pr-subscribers-libc Author: None (Sterling-Augustine) ChangesThis options sets a compile option when building sources inside the string directory, and this option affects string_utils.h. But string_utils.h is #included from more places than just the string directory (such as from __support/CPP/string.h), leading to both narrow-reads in those cases, but more seriously, ODR violations when the two different string_length implementations are included int he same program. Having this option at the top level avoids this problem. Full diff: https://github.com/llvm/llvm-project/pull/165046.diff 2 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 4c36ed8620f40..c4e4a9e3465d7 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -81,6 +81,10 @@ function(_get_compile_options_from_config output_var)
list(APPEND config_options "-DLIBC_QSORT_IMPL=${LIBC_CONF_QSORT_IMPL}")
endif()
+ if(LIBC_CONF_STRING_UNSAFE_WIDE_READ)
+ list(APPEND config_options "-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
+ endif()
+
if(LIBC_TYPES_TIME_T_IS_32_BIT AND LLVM_LIBC_FULL_BUILD)
list(APPEND config_options "-DLIBC_TYPES_TIME_T_IS_32_BIT")
endif()
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index 83c956429be24..fd1ae8d417f8c 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -1,8 +1,5 @@
add_subdirectory(memory_utils)
-if(LIBC_CONF_STRING_UNSAFE_WIDE_READ)
- list(APPEND string_config_options "-DLIBC_COPT_STRING_UNSAFE_WIDE_READ")
-endif()
if(LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING)
list(APPEND string_config_options "-DLIBC_COPT_MEMSET_X86_USE_SOFTWARE_PREFETCHING")
endif()
|
libc/src/string/CMakeLists.txt
Outdated
| if(LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING) | ||
| list(APPEND string_config_options "-DLIBC_COPT_MEMSET_X86_USE_SOFTWARE_PREFETCHING") | ||
| endif() |
There was a problem hiding this comment.
since you're here, do you mind also moving this config to top-level and removing string_config_options here? Thanks,
libc/src/string/CMakeLists.txt
Outdated
| if(string_config_options) | ||
| list(PREPEND string_config_options "COMPILE_OPTIONS") | ||
| endif() |
There was a problem hiding this comment.
nit: Remove this if() ... endif() and ${string_config_options} in add_header_library(string_utils ... ) below.
4 participants
This options sets a compile option when building sources inside the string directory, and this option affects string_utils.h. But string_utils.h is #included from more places than just the string directory (such as from __support/CPP/string.h), leading to both narrow-reads in those cases, but more seriously, ODR violations when the two different string_length implementations are included int he same program.
Having this option at the top level avoids this problem.