[libc++] Allow for hardening in multiple categories #79859
Conversation
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Will Hawkins (hawkinsw) ChangesAllow multiple categories for hardening assertions. Full diff: https://github.com/llvm/llvm-project/pull/79859.diff 2 Files Affected:
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 9fc608ee14320d..36e0a98d111724 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -342,6 +342,15 @@ _LIBCPP_HARDENING_MODE_DEBUG
// clang-format off
// Fast hardening mode checks.
+// List all checks
+
+// VALID_INPUT_RANGE: Description of reason to use the check.
+// VALID_ELEMENT_ACCESS: Description of reason to use the check.
+// COMPATIBLE_ALLOCATOR: Description of reason to use the check.
+// PEDANTIC: Description of reason to use the check.
+// INTERNAL: Description of reason to use the check.
+// UNCATEGORIZED: Description of reason to use the check.
+
# if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_FAST
// Enabled checks.
@@ -362,6 +371,16 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
+# define VALID_INPUT_RANGE 1
+# define VALID_PRECONDITIONS 1
+# define VALID_ELEMENT_ACCESS 1
+# define NON_NULL 0
+# define OVERLAPPING_RANGES 0
+# define COMPATIBLE_ALLOCATOR 0
+# define PEDANTIC 0
+# define INTERNAL 0
+# define UNCATEGORIZED 0
+
// Extensive hardening mode checks.
# elif _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_EXTENSIVE
@@ -381,6 +400,17 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
+# define VALID_INPUT_RANGE 1
+# define VALID_ELEMENT_ACCESS 1
+# define VALID_PRECONDITIONS 1
+# define NON_NULL 1
+# define OVERLAPPING_RANGES 1
+# define COMPATIBLE_ALLOCATOR 1
+# define PEDANTIC 1
+# define UNCATEGORIZED 1
+
+# define INTERNAL 0
+
// Debug hardening mode checks.
# elif _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG
@@ -399,6 +429,16 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
+# define VALID_INPUT_RANGE 1
+# define VALID_ELEMENT_ACCESS 1
+# define VALID_PRECONDITIONS 1
+# define NON_NULL 1
+# define OVERLAPPING_RANGES 1
+# define COMPATIBLE_ALLOCATOR 1
+# define PEDANTIC 1
+# define INTERNAL 1
+# define UNCATEGORIZED 1
+
// Disable all checks if hardening is not enabled.
# else
@@ -417,7 +457,27 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
+# define VALID_INPUT_RANGE 0
+# define VALID_ELEMENT_ACCESS 0
+# define VALID_PRECONDITIONS 0
+# define NON_NULL 0
+# define OVERLAPPING_RANGES 0
+# define COMPATIBLE_ALLOCATOR 0
+# define PEDANTIC 0
+# define INTERNAL 0
+# define UNCATEGORIZED 0
+
# endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_FAST
+
+#define _LIBCPP_ASSERT_P(reason, expression, message) \
+do { \
+ if (reason) { \
+ _LIBCPP_ASSERT(expression, message); \
+ } else { \
+ _LIBCPP_ASSUME(expression); \
+ } \
+} while (0)
+
// clang-format on
// } HARDENING
diff --git a/libcxx/include/__iterator/counted_iterator.h b/libcxx/include/__iterator/counted_iterator.h
index 008c52fa87ce00..58f4317f0ba713 100644
--- a/libcxx/include/__iterator/counted_iterator.h
+++ b/libcxx/include/__iterator/counted_iterator.h
@@ -229,7 +229,7 @@ class counted_iterator
_LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator[](iter_difference_t<_Iter> __n) const
requires random_access_iterator<_Iter>
{
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__n < __count_, "Subscript argument must be less than size.");
+ _LIBCPP_ASSERT_P(VALID_ELEMENT_ACCESS|VALID_PRECONDITIONS, __n < __count_, "Subscript argument must be less than size.");
return __current_[__n];
}
|
You can test this locally with the following command:git-clang-format --diff 9520773c46777adbc1d489f831d6c93b8287ca0e 5353b7fb9a8b1fefa9f139b8cc29209ef7f9d575 -- libcxx/include/__assert libcxx/include/__config libcxx/include/__iterator/counted_iterator.hView the diff from clang-format here.diff --git a/libcxx/include/__iterator/counted_iterator.h b/libcxx/include/__iterator/counted_iterator.h
index 8f9bc1a9d4..70f6d179fe 100644
--- a/libcxx/include/__iterator/counted_iterator.h
+++ b/libcxx/include/__iterator/counted_iterator.h
@@ -229,7 +229,9 @@ public:
_LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator[](iter_difference_t<_Iter> __n) const
requires random_access_iterator<_Iter>
{
- _LIBCPP_ASSERT_P(_HD_VALID_ELEMENT_ACCESS|_HD_VALID_PRECONDITIONS, __n < __count_, "Subscript argument must be less than size.");
+ _LIBCPP_ASSERT_P(_HD_VALID_ELEMENT_ACCESS | _HD_VALID_PRECONDITIONS,
+ __n < __count_,
+ "Subscript argument must be less than size.");
return __current_[__n];
}
|
|
@var-const I thought it would be cool to allow for assertions to be in multiple categories. I worked up a quick sample of how that could look. Obviously I didn't go too far because I wanted to get your thoughts. There are a ton of open questions about this (not the least of which is whether or not the macro would always compile to 0 overhead). But, I thought it might be a cool option to be able to have assertions in multiple categories. If you want me to continue to iterate, please let me know! I love helping! Will |
Allow multiple categories for hardening assertions.
hawkinsw
force-pushed
the
multi_hardening
branch
from
f7ad541 to
c244346
Compare
Revamp underlying plumbing.
|
@var-const Spent lots of time attempting to make this better but didn't get very far. I made a few minor tweaks. I hope that this is something that might be of use. |
|
@var-const Hope things are going well. If you think something like this is useful/interesting, just let me know and I will attempt to pursue it! |
|
(@hawkinsw Very sorry about the slow reply, I'm just catching up on the reviews) It's an interesting idea, thanks for working on this! Personally, I would postpone this until we have a use case. The current model we have, where assertions are classified across a single axis (even if that axis takes multiple factors into account), while perhaps limited, has the advantage of being simple. That makes understanding which assertion categories go into which modes easier. If we start running into cases where it just feels natural to start essentially tagging assertions, or if we come up with use cases where people want modes grouped around orthogonal criteria, we could revisit this patch and use this mechanism to implement that. Until we have specific use cases, however, I think let's try to keep things as simple as possible. The less modes we support, the easier it is for us to reason about what is the exact effect of each mode and what kind of (best effort) guarantees it provides. |
As I mentioned in DM with you, please do not apologize! I appreciate and agree with everything that you have said here. I will pull back this PR until such time as we would like to have the capability to harden over multiple categories. I was suggesting it really to get your feedback on its technical merit. So, thank you for validating that the technical would be valid. Thank you for all you do! |
Allow multiple categories for hardening assertions.