[libc-commits] [libc] [libc][cmake] reset COMPILE_DEFINITIONS (PR #77810)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

<details>
<summary>Changes</summary>

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


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


3 Files Affected:

- (modified) libc/CMakeLists.txt (+9-4) 
- (modified) libc/src/sys/mman/linux/mmap.cpp (+2) 
- (modified) libc/src/sys/mman/mmap.h (+2-1) 


``````````diff
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 9c82db754b5f73..6ff571aa0ae85a 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)
diff --git a/libc/src/sys/mman/linux/mmap.cpp b/libc/src/sys/mman/linux/mmap.cpp
index 16111c66859f5e..4cfed24b7b90da 100644
--- a/libc/src/sys/mman/linux/mmap.cpp
+++ b/libc/src/sys/mman/linux/mmap.cpp
@@ -12,6 +12,8 @@
 #include "src/__support/common.h"
 
 #include "src/errno/libc_errno.h"
+#include "llvm-libc-macros/sys-mman-macros.h" // For MAP_FAILED.
+
 #include <linux/param.h> // For EXEC_PAGESIZE.
 #include <sys/syscall.h> // For syscall numbers.
 
diff --git a/libc/src/sys/mman/mmap.h b/libc/src/sys/mman/mmap.h
index 4425019c4ee1a1..10f30d895b3e35 100644
--- a/libc/src/sys/mman/mmap.h
+++ b/libc/src/sys/mman/mmap.h
@@ -9,7 +9,8 @@
 #ifndef LLVM_LIBC_SRC_SYS_MMAN_MMAP_H
 #define LLVM_LIBC_SRC_SYS_MMAN_MMAP_H
 
-#include <sys/mman.h> // For size_t and off_t
+#include "llvm-libc-types/off_t.h"
+#include "llvm-libc-types/size_t.h"
 
 namespace LIBC_NAMESPACE {
 

``````````

</details>


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