-
Notifications
You must be signed in to change notification settings - Fork 18.1k
[libc++][hardening] Rework how the assertion handler can be overridden. #77883
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
Changes from 7 commits
e8e95c5
86fc352
216605f
692aac0
8981ef9
50282f3
deae2b8
d38fe7e
dc9d771
d83f5a1
48db22a
230de80
5dd7588
b6bcc16
1f5cab9
0bef0fb
cfa7d34
6cb68e6
8cc2fff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // -*- C++ -*- | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to prefix this filename with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to, if it gets installed into
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I mean this specific file. During installation, it gets renamed to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I would leave it without |
||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
|
var-const marked this conversation as resolved.
|
||
| #ifndef _LIBCPP___ASSERTION_HANDLER | ||
| #define _LIBCPP___ASSERTION_HANDLER | ||
|
|
||
| #include <__config> | ||
| #include <__verbose_abort> | ||
|
|
||
| #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) | ||
|
var-const marked this conversation as resolved.
|
||
| # pragma GCC system_header | ||
| #endif | ||
|
|
||
| // TODO(hardening): in production, trap rather than abort. | ||
| #define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_ABORT(message) | ||
|
var-const marked this conversation as resolved.
Outdated
|
||
|
|
||
| #endif // _LIBCPP___ASSERTION_HANDLER | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I first tried using
file(COPY ...), but there seem to be several downsides to it:file(COPY_FILE ...)command does, but it's only available starting from CMake3.21, which is above the minimum version we support (3.20). I tried renaming after copying, but it looks pretty ugly:(It is said about
COPY_FILEbut seems to apply toCOPYas well)This seems to be a downside. Testing locally, I can confirm that modifying the header triggers a rebuild of
cxx-test-dependsif I useconfigure_filebut not if I usefile(COPY ...).IIUC, the
COPYONLYoption prevents variables from being expanded, making the behavior identical. Let me know if I'm missing anything!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Tagging @mordante to make sure this comment doesn't get lost among various threads)