Skip to content
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

[libc] set -Wno-frame-address for thread.cpp #77140

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers commented Jan 5, 2024

The aarch64 code is using __builtin_return_address with a non-zero parameter,
which generates the following warning:

    llvm-project/libc/src/__support/threads/linux/thread.cpp:171:38: error:
    calling '__builtin_frame_address' with a nonzero argument is unsafe
    [-Werror,-Wframe-address]
      171 |   return reinterpret_cast<uintptr_t>(__builtin_frame_address(1));
          |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~

Disable this diagnostic just for this file so that we can enable -Werror.

Fixes: #77007

@llvmbot llvmbot added the libc label Jan 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes
  • [libc][cmake] append per obj compile options instead of prepending
  • [libc] set -Wno-frame-address for thread.cpp

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

2 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCObjectRules.cmake (+5-11)
  • (modified) libc/src/__support/threads/linux/CMakeLists.txt (+2)
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 5fbbfd58db2d07..c3e3fa2dccfe53 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -26,7 +26,7 @@ function(_get_common_compile_options output_var flags)
     set(ADD_PREFER_GENERIC_FLAG TRUE)
   endif()
 
-  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT} ${ARGN})
+  set(compile_options ${LIBC_COMPILE_OPTIONS_DEFAULT})
   if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
     list(APPEND compile_options "-fpie")
 
@@ -357,11 +357,8 @@ function(create_object_library fq_target_name)
     set(internal_target_name ${fq_target_name})
   endif()
 
-  _get_common_compile_options(
-    compile_options
-    "${ADD_OBJECT_FLAGS}"
-    ${ADD_OBJECT_COMPILE_OPTIONS}
-  )
+  _get_common_compile_options(compile_options "${ADD_OBJECT_FLAGS}")
+  list(APPEND compile_options ${ADD_OBJECT_COMPILE_OPTIONS})
 
   # GPU builds require special handling for the objects because we want to
   # export several different targets at once, e.g. for both Nvidia and AMD.
@@ -640,11 +637,8 @@ function(create_entrypoint_object fq_target_name)
     set(ADD_ENTRYPOINT_OBJ_CXX_STANDARD ${CMAKE_CXX_STANDARD})
   endif()
 
-  _get_common_compile_options(
-    common_compile_options
-    "${ADD_ENTRYPOINT_OBJ_FLAGS}"
-    ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS}
-  )
+  _get_common_compile_options(common_compile_options "${ADD_ENTRYPOINT_OBJ_FLAGS}")
+  list(APPEND common_compile_options ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS})
   get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
   set(full_deps_list ${fq_deps_list} libc.src.__support.common)
 
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 642eead7277262..148a0ba061c577 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -39,6 +39,8 @@ add_object_library(
     -O3
     -fno-omit-frame-pointer # This allows us to sniff out the thread args from
                             # the new thread's stack reliably.
+    -Wno-frame-address      # Yes, calling __builtin_return_address with a
+                            # value other than 0 is dangerous. We know.
 )
 
 add_object_library(

@nickdesaulniers nickdesaulniers changed the title arm64 frame address [libc] set -Wno-frame-address for thread.cpp Jan 5, 2024
The aarch64 code is using __builtin_return_address with a non-zero parameter,
which generates the following warning:

    llvm-project/libc/src/__support/threads/linux/thread.cpp:171:38: error:
    calling '__builtin_frame_address' with a nonzero argument is unsafe
    [-Werror,-Wframe-address]
      171 |   return reinterpret_cast<uintptr_t>(__builtin_frame_address(1));
          |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~

Disable this diagnostic just for this file so that we can enable -Werror.

Fixes: llvm#77007
@nickdesaulniers nickdesaulniers merged commit 12101ca into llvm:main Jan 8, 2024
4 checks passed
@nickdesaulniers nickdesaulniers deleted the arm64_frame_address branch January 8, 2024 16:51
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
The aarch64 code is using __builtin_return_address with a non-zero
parameter,
which generates the following warning:

llvm-project/libc/src/__support/threads/linux/thread.cpp:171:38: error:
calling '__builtin_frame_address' with a nonzero argument is unsafe
        [-Werror,-Wframe-address]
171 | return reinterpret_cast<uintptr_t>(__builtin_frame_address(1));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
    
Disable this diagnostic just for this file so that we can enable
-Werror.
    
Fixes: llvm#77007
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.

[libc] -Wframe-address in libc/src/__support/threads/linux/thread.cpp
3 participants