Skip to content

Reland "[libc] fix aarch64 linux full build (#95358)" #95423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

SchrodingerZhu
Copy link
Contributor

@SchrodingerZhu SchrodingerZhu commented Jun 13, 2024

Reverts #95419 and Reland #95358.

This PR is full of temporal fixes. After a discussion with @lntue, it is better to avoid further changes to the cmake infrastructure for now as a rework to the cmake utilities will be landed in the future.

@SchrodingerZhu SchrodingerZhu marked this pull request as ready for review June 14, 2024 03:29
@SchrodingerZhu SchrodingerZhu requested a review from lntue June 14, 2024 03:29
@llvmbot llvmbot added the libc label Jun 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 14, 2024

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Reverts llvm/llvm-project#95419 and Reland #95358 (WIP)


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

7 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+9)
  • (modified) libc/config/linux/aarch64/entrypoints.txt (+7)
  • (modified) libc/src/__support/threads/linux/CMakeLists.txt (+1)
  • (modified) libc/test/IntegrationTest/CMakeLists.txt (+5)
  • (modified) libc/test/IntegrationTest/test.cpp (+10)
  • (modified) libc/test/UnitTest/CMakeLists.txt (+1-1)
  • (modified) libc/test/UnitTest/HermeticTestUtils.cpp (+16)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index eb6be91b55e26..c8d7c8a2b1c7c 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -686,6 +686,15 @@ function(add_libc_hermetic_test test_name)
                    LibcTest.hermetic
                    libc.test.UnitTest.ErrnoSetterMatcher
                    ${fq_deps_list})
+  # TODO: currently the dependency chain is broken such that getauxval cannot properly
+  # propagate to hermetic tests. This is a temporary workaround.
+  if (LIBC_TARGET_ARCHITECTURE_IS_AARCH64)
+    target_link_libraries(
+      ${fq_build_target_name}
+      PRIVATE
+        libc.src.sys.auxv.getauxval
+    )
+  endif()
 
   # Tests on the GPU require an external loader utility to launch the kernel.
   if(TARGET libc.utils.gpu.loader)
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index db96a80051a8d..7ce088689b925 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -643,6 +643,12 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.pthread.pthread_mutexattr_setrobust
     libc.src.pthread.pthread_mutexattr_settype
     libc.src.pthread.pthread_once
+    libc.src.pthread.pthread_rwlockattr_destroy
+    libc.src.pthread.pthread_rwlockattr_getkind_np
+    libc.src.pthread.pthread_rwlockattr_getpshared
+    libc.src.pthread.pthread_rwlockattr_init
+    libc.src.pthread.pthread_rwlockattr_setkind_np
+    libc.src.pthread.pthread_rwlockattr_setpshared
     libc.src.pthread.pthread_setspecific
 
     # sched.h entrypoints
@@ -753,6 +759,7 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.unistd._exit
     libc.src.unistd.environ
     libc.src.unistd.execv
+    libc.src.unistd.fork
     libc.src.unistd.getopt
     libc.src.unistd.optarg
     libc.src.unistd.optind
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 9bf88ccc84557..8e6cd7227b2c8 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -64,6 +64,7 @@ add_object_library(
     .futex_utils
     libc.config.linux.app_h
     libc.include.sys_syscall
+    libc.include.fcntl
     libc.src.errno.errno
     libc.src.__support.CPP.atomic
     libc.src.__support.CPP.stringstream
diff --git a/libc/test/IntegrationTest/CMakeLists.txt b/libc/test/IntegrationTest/CMakeLists.txt
index 4f31f10b29f0b..4a999407d48d7 100644
--- a/libc/test/IntegrationTest/CMakeLists.txt
+++ b/libc/test/IntegrationTest/CMakeLists.txt
@@ -1,3 +1,7 @@
+set(arch_specific_deps)
+if(LIBC_TARGET_ARCHITECTURE_IS_AARCH64)
+  set(arch_specific_deps libc.src.sys.auxv.getauxval)
+endif()
 add_object_library(
   test
   SRCS
@@ -8,4 +12,5 @@ add_object_library(
     test.h
   DEPENDS
     libc.src.__support.OSUtil.osutil
+    ${arch_specific_deps}
 )
diff --git a/libc/test/IntegrationTest/test.cpp b/libc/test/IntegrationTest/test.cpp
index 3bdbe89a3fb62..a8b2f2911fd8e 100644
--- a/libc/test/IntegrationTest/test.cpp
+++ b/libc/test/IntegrationTest/test.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/__support/common.h"
+#include "src/sys/auxv/getauxval.h"
 #include <stddef.h>
 #include <stdint.h>
 
@@ -79,4 +81,12 @@ void *realloc(void *ptr, size_t s) {
 // Integration tests are linked with -nostdlib. BFD linker expects
 // __dso_handle when -nostdlib is used.
 void *__dso_handle = nullptr;
+
+#ifdef LIBC_TARGET_ARCH_IS_AARCH64
+// Due to historical reasons, libgcc on aarch64 may expect __getauxval to be
+// defined. See also https://gcc.gnu.org/pipermail/gcc-cvs/2020-June/300635.html
+unsigned long __getauxval(unsigned long id) {
+  return LIBC_NAMESPACE::getauxval(id);
+}
+#endif
 } // extern "C"
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index 302af3044ca3d..4adc2f5c725f7 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -41,7 +41,7 @@ function(add_unittest_framework_library name)
   target_compile_options(${name}.hermetic PRIVATE ${compile_options})
 
   if(TEST_LIB_DEPENDS)
-    foreach(dep IN LISTS ${TEST_LIB_DEPENDS})
+    foreach(dep IN ITEMS ${TEST_LIB_DEPENDS})
       if(TARGET ${dep}.unit)
         add_dependencies(${name}.unit ${dep}.unit)
       else()
diff --git a/libc/test/UnitTest/HermeticTestUtils.cpp b/libc/test/UnitTest/HermeticTestUtils.cpp
index 349c182ff2379..6e815e6c8aab0 100644
--- a/libc/test/UnitTest/HermeticTestUtils.cpp
+++ b/libc/test/UnitTest/HermeticTestUtils.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/__support/common.h"
+#include "src/sys/auxv/getauxval.h"
 #include <stddef.h>
 #include <stdint.h>
 
@@ -19,6 +21,12 @@ void *memmove(void *dst, const void *src, size_t count);
 void *memset(void *ptr, int value, size_t count);
 int atexit(void (*func)(void));
 
+// TODO: It seems that some old test frameworks does not use
+// add_libc_hermetic_test properly. Such that they won't get correct linkage
+// against the object containing this function. We create a dummy function that
+// always returns 0 to indicate a failure.
+[[gnu::weak]] unsigned long getauxval(unsigned long id) { return 0; }
+
 } // namespace LIBC_NAMESPACE
 
 namespace {
@@ -102,6 +110,14 @@ void __cxa_pure_virtual() {
 // __dso_handle when -nostdlib is used.
 void *__dso_handle = nullptr;
 
+#ifdef LIBC_TARGET_ARCH_IS_AARCH64
+// Due to historical reasons, libgcc on aarch64 may expect __getauxval to be
+// defined. See also https://gcc.gnu.org/pipermail/gcc-cvs/2020-June/300635.html
+unsigned long __getauxval(unsigned long id) {
+  return LIBC_NAMESPACE::getauxval(id);
+}
+#endif
+
 } // extern "C"
 
 void *operator new(unsigned long size, void *ptr) { return ptr; }

@SchrodingerZhu SchrodingerZhu merged commit 2efe3d7 into main Jun 14, 2024
7 of 8 checks passed
@SchrodingerZhu SchrodingerZhu deleted the revert-95419-revert-95358-libc/aarch64-linux-fix branch June 14, 2024 03:34
EthanLuisMcDonough pushed a commit to EthanLuisMcDonough/llvm-project that referenced this pull request Aug 13, 2024
Reverts llvm#95419 and Reland llvm#95358.

This PR is full of temporal fixes. After a discussion with @lntue, it is
better to avoid further changes to the cmake infrastructure for now as a
rework to the cmake utilities will be landed in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants