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++abi] Replace usage of raw assert by _LIBCXXABI_ASSERT #80689

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Feb 5, 2024

We strive not to use raw assert(...) anymore in libc++abi in preparation for using the hardening framework.

We strive not to use raw assert(...) anymore in libc++abi in preparation
for using the hardening framework.
@ldionne ldionne requested a review from a team as a code owner February 5, 2024 14:44
@llvmbot llvmbot added the libc++abi libc++abi C++ Runtime Library. Not libc++. label Feb 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2024

@llvm/pr-subscribers-libcxxabi

Author: Louis Dionne (ldionne)

Changes

We strive not to use raw assert(...) anymore in libc++abi in preparation for using the hardening framework.


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

1 Files Affected:

  • (modified) libcxxabi/src/private_typeinfo.cpp (+2-2)
diff --git a/libcxxabi/src/private_typeinfo.cpp b/libcxxabi/src/private_typeinfo.cpp
index 857ae25b7028d..5c68f3e994cd9 100644
--- a/libcxxabi/src/private_typeinfo.cpp
+++ b/libcxxabi/src/private_typeinfo.cpp
@@ -44,9 +44,9 @@
 #include <cstdint>
 #include <cassert>
 #include <string.h>
+#include "abort_message.h"
 
 #ifdef _LIBCXXABI_FORGIVING_DYNAMIC_CAST
-#include "abort_message.h"
 #include <sys/syslog.h>
 #include <atomic>
 #endif
@@ -470,7 +470,7 @@ __class_type_info::can_catch(const __shim_type_info* thrown_type,
     if (thrown_class_type == 0)
         return false;
     // bullet 2
-    assert(adjustedPtr && "catching a class without an object?");
+    _LIBCXXABI_ASSERT(adjustedPtr, "catching a class without an object?");
     __dynamic_cast_info info = {thrown_class_type, 0, this, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, true, nullptr};
     info.number_of_dst_type = 1;
     thrown_class_type->has_unambiguous_public_base(&info, adjustedPtr, public_path);

Copy link

github-actions bot commented Feb 5, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff daea0820829bf5bbca9ab50fc118012a2508fab3 cca54ee4ba6813c17fac3550340ac3c79cbe8186 -- libcxxabi/src/private_typeinfo.cpp
View the diff from clang-format here.
diff --git a/libcxxabi/src/private_typeinfo.cpp b/libcxxabi/src/private_typeinfo.cpp
index 5c68f3e994..47325494ff 100644
--- a/libcxxabi/src/private_typeinfo.cpp
+++ b/libcxxabi/src/private_typeinfo.cpp
@@ -47,8 +47,8 @@
 #include "abort_message.h"
 
 #ifdef _LIBCXXABI_FORGIVING_DYNAMIC_CAST
-#include <sys/syslog.h>
-#include <atomic>
+#  include <sys/syslog.h>
+#  include <atomic>
 #endif
 
 static inline

@ldionne ldionne merged commit 58f3a77 into llvm:main Feb 5, 2024
@ldionne ldionne deleted the review/libcxxabi-remove-cassert branch February 5, 2024 16:21
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
)

We strive not to use raw assert(...) anymore in libc++abi in preparation
for using the hardening framework.
aheejin added a commit to emscripten-core/emscripten that referenced this pull request Jan 3, 2025
This updates libcxx and libcxxabi to LLVM 19.1.4:
https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.4

The initial update was done using `update_libcxx.py` and
`update_libcxxabi.py`, and subsequent fixes were made in indidual
commits. The commit history here is kind of messy because of CI testing
so not all individual commits are noteworthy.

Additional changes:

- Build libcxx and libcxxabi with C++23:
8b0bfdf

https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp
was added in llvm/llvm-project#87390 and this
file assumes C++23 to be compiled. Apparently libc++ sources are always
built with C++23 so they don't guard things against it in `src/`:
llvm/llvm-project#87390 (comment)
This commit also builds libc++abi with C++23 because it doesn't seem
there's any downside to it.

- Exclude newly added `compiler_rt_shims.cpp`:
5bbcbf0
We have excluded files in
https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32.
This is a new file added in this directory in
llvm/llvm-project#83575.

- Disable time zone support:
a5f2cbe
We disabled C++20 time zone support in LLVM 18 update (#21638):
df9af64
The list of source files related to time zone support has changed in
llvm/llvm-project#74928, so this commit reflects
it.

- Re-add + update `__assertion_handler` from
`default_assertion_handler.in`:
41f8037
This file was added as a part of LLVM 18 update (#21638) in
8d51927
and mistakenly deleted when I ran `update_libcxx.py`. This file was
copied from
https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in,
so this also updates the file with the newest
`default_assertion_handler.in`.

- `_LIBCPP_PSTL_CPU_BACKEND_SERIAL` -> `_LIBCPP_PSTL_BACKEND_SERIAL`:
4b969c3
The name changed in this update, so reflecting it on our
`__config_site`.

- Directly include `pthread.h` from `emscripten/val.h`:
a5a76c3
  Due to file rearrangements happened, this was necessary.

---

Other things to note:

- `std::basic_string<unsigned_char>` is not supported anymore
The support for `std::basic_string<unsigned_char>`, which was being used
by embind, is removed in this version.#23070 removes the uses from
embind.

- libcxxabi uses `__FILE__` in more places
llvm/llvm-project#80689 started to use
`_LIBCXXABI_ASSERT`, which
[uses](https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxxabi/src/abort_message.h#L22)
`__FILE__`, in `private_typeinfo.cpp`.
`__FILE__` macro produces different paths depending on the local
directory names and how the file is given to clang in the command line,
and this file is included in the result of one of our code size tests,
`other.test_minimal_runtime_code_size_hello_embind`, which caused the
result of the test to vary depending on the CI bots and how the library
is built (e.g., whether by embuilder, ninja, or neither).
Even though this was brought to surface during this LLVM 19 update,
`__FILE__` macro could be a problem for producing reproducible builds
anyway. We discussed this problem in #23195, and the fixes landed in
#23222, #23225, and #23256.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants