Skip to content

[libc++] Disable CFI in __libcpp_allocate #124805

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 1 commit into from
Jan 28, 2025

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jan 28, 2025

Since we're casting uninitialized memory, we must disable CFI checks.

Since we're casting uninitialized memory, we must disable CFI checks.
@ldionne ldionne requested a review from a team as a code owner January 28, 2025 17:51
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 28, 2025
@ldionne ldionne added the pending-ci Merging the PR is only pending completion of CI label Jan 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 28, 2025

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Since we're casting uninitialized memory, we must disable CFI checks.


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

1 Files Affected:

  • (modified) libcxx/include/__new/allocate.h (+2-1)
diff --git a/libcxx/include/__new/allocate.h b/libcxx/include/__new/allocate.h
index a64663c09fa35d..738fa62af4d61d 100644
--- a/libcxx/include/__new/allocate.h
+++ b/libcxx/include/__new/allocate.h
@@ -50,7 +50,8 @@ _LIBCPP_HIDE_FROM_ABI void __libcpp_operator_delete(_Args... __args) _NOEXCEPT {
 }
 
 template <class _Tp>
-inline _LIBCPP_HIDE_FROM_ABI _Tp* __libcpp_allocate(__element_count __n, size_t __align = _LIBCPP_ALIGNOF(_Tp)) {
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _Tp*
+__libcpp_allocate(__element_count __n, size_t __align = _LIBCPP_ALIGNOF(_Tp)) {
   size_t __size = static_cast<size_t>(__n) * sizeof(_Tp);
 #if _LIBCPP_HAS_ALIGNED_ALLOCATION
   if (__is_overaligned_for_new(__align)) {

@ldionne ldionne merged commit 9f66062 into llvm:main Jan 28, 2025
84 checks passed
@ldionne ldionne deleted the review/libcpp-allocate-no-cfi branch January 28, 2025 20:43
@philnik777
Copy link
Contributor

@ldionne Could we maybe look into adding a bot for CFI? It's not the first time there is some issue with it, and I don't think there's a fundamental problem with enabling it.

@philnik777
Copy link
Contributor

Looks like you're already working on it in #124837 :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. pending-ci Merging the PR is only pending completion of CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants