From 32b592ada66dbd0ec8b6adaa60173252421c1265 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Thu, 11 Jan 2024 09:50:47 -0800 Subject: [PATCH 1/2] [libc][cmake] reset COMPILE_DEFINITIONS While trying to enable -Werror (#74506), the 32b ARM build bot reported an error stemming from -Wshorten-64-to-32 related to usages of `off_t`. I failed to fix these properly in #77350 (the 32b ARM build is not a fullbuild) and #77396. It turns out, the preprocessor defines `-D_LARGEFILE_SOURCE` and `-D_FILE_OFFSET_BITS=64` were being set for llvmlibc when using the cmake build system. In particular, these preprocessor defines are feature test macros used by glibc, and which have effects no the corresponding ABI for types like `off_t` (for instance, should `off_t` be 32b or 64b on 32b targets). But who was setting these? Turns out that the use of add_compile_definitions in llvm/cmake/modules/HandleLLVMOptions.cmake was setting these (and more), which is then inherited by every subdirectory. While some of these defines maybe make sense for host builds, they do not make sense for libraries for the target. The full list of defines being set prior to this commit: - `-D_GNU_SOURCE` - `-D_FILE_OFFSET_BITS=64` - `-D_DEBUG` - `-D_GLIBCXX_ASSERTIONS` - `-D_LARGEFILE_SOURCE` - `-D_FILE_OFFSET_BITS=64` - `-D__STDC_CONSTANT_MACROS` - `-D__STDC_FORMAT_MACROS` - `-D__STDC_LIMIT_MACROS` If we desire any of the above, we should manually reset them. Fix this by resetting COMPILE_DEFINITIONS for the libc/ subdirectory. Side note: to debug 'directory properties' in cmake, you first need to use `get_directory_property` to fetch the corresponding value into a variable first, then that variable can be printed via `message`. Link: https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html#index-_005fFILE_005fOFFSET_005fBITS Link: https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html#index-_005fFILE_005fOFFSET_005fBITS Fixes: #77395 --- libc/CMakeLists.txt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt index 9c82db754b5f7..6ff571aa0ae85 100644 --- a/libc/CMakeLists.txt +++ b/libc/CMakeLists.txt @@ -7,10 +7,15 @@ endif() include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake NO_POLICY_SCOPE) -# `llvm-project/llvm/CMakeLists.txt` adds the following directive -# `include_directories( ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR})` -# We undo it to be able to precisely control what is getting included. -set_directory_properties(PROPERTIES INCLUDE_DIRECTORIES "") +set_directory_properties(PROPERTIES + # `llvm-project/llvm/CMakeLists.txt` adds the following directive + # `include_directories( ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR})` We + # undo it to be able to precisely control what is getting included. + INCLUDE_DIRECTORIES "" + # `llvm/cmake/modules/HandleLLVMOptions.cmake` uses `add_compile_definitions` + # to set a few preprocessor defines which we do not want. + COMPILE_DEFINITIONS "" +) # Default to C++17 set(CMAKE_CXX_STANDARD 17) From fdc1e357266e1e5fc8c2c72cf3ef13350d0ee79e Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Thu, 11 Jan 2024 13:01:59 -0800 Subject: [PATCH 2/2] re-add _DEBUG for Debug builds, and print undefines in verbose mode --- libc/CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt index 6ff571aa0ae85..1cd00ed9aa7c4 100644 --- a/libc/CMakeLists.txt +++ b/libc/CMakeLists.txt @@ -7,6 +7,12 @@ endif() include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake NO_POLICY_SCOPE) +if (LIBC_CMAKE_VERBOSE_LOGGING) + get_directory_property(LIBC_OLD_PREPROCESSOR_DEFS COMPILE_DEFINITIONS) + foreach(OLD_DEF ${LIBC_OLD_PREPROCESSOR_DEFS}) + message(STATUS "Undefining ${OLD_DEF}") + endforeach() +endif() set_directory_properties(PROPERTIES # `llvm-project/llvm/CMakeLists.txt` adds the following directive # `include_directories( ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR})` We @@ -16,6 +22,9 @@ set_directory_properties(PROPERTIES # to set a few preprocessor defines which we do not want. COMPILE_DEFINITIONS "" ) +if (CMAKE_BUILD_TYPE STREQUAL "Debug") + add_definitions("-D_DEBUG") +endif() # Default to C++17 set(CMAKE_CXX_STANDARD 17)