Skip to content

Commit dc57752

Browse files
authored
[libc++][hardening] Categorize assertions that produce incorrect results (#77183)
Introduce a new `argument-within-domain` category that covers cases where the given arguments make it impossible to produce a correct result (or create a valid object in case of constructors). While the incorrect result doesn't create an immediate problem within the library (like e.g. a null pointer dereference would), it always indicates a logic error in user code and is highly likely to lead to a bug in the program once the value is used.
1 parent f0c920f commit dc57752

File tree

13 files changed

+44
-29
lines changed

13 files changed

+44
-29
lines changed

libcxx/include/__algorithm/clamp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ clamp(_LIBCPP_LIFETIMEBOUND const _Tp& __v,
2626
_LIBCPP_LIFETIMEBOUND const _Tp& __lo,
2727
_LIBCPP_LIFETIMEBOUND const _Tp& __hi,
2828
_Compare __comp) {
29-
_LIBCPP_ASSERT_UNCATEGORIZED(!__comp(__hi, __lo), "Bad bounds passed to std::clamp");
29+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(!__comp(__hi, __lo), "Bad bounds passed to std::clamp");
3030
return __comp(__v, __lo) ? __lo : __comp(__hi, __v) ? __hi : __v;
3131
}
3232

libcxx/include/__algorithm/ranges_clamp.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ struct __fn {
3434
indirect_strict_weak_order<projected<const _Type*, _Proj>> _Comp = ranges::less>
3535
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr const _Type& operator()(
3636
const _Type& __value, const _Type& __low, const _Type& __high, _Comp __comp = {}, _Proj __proj = {}) const {
37-
_LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
38-
"Bad bounds passed to std::ranges::clamp");
37+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
38+
!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
39+
"Bad bounds passed to std::ranges::clamp");
3940

4041
auto&& __projected = std::invoke(__proj, __value);
4142
if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))

libcxx/include/__bit/bit_ceil.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp __bit_ceil(_Tp __t) no
2828
if (__t < 2)
2929
return 1;
3030
const unsigned __n = numeric_limits<_Tp>::digits - std::__countl_zero((_Tp)(__t - 1u));
31-
_LIBCPP_ASSERT_UNCATEGORIZED(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
31+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
3232

3333
if constexpr (sizeof(_Tp) >= sizeof(unsigned))
3434
return _Tp{1} << __n;

libcxx/include/__config

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,13 @@
294294
// - `_LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR` -- checks any operations that exchange nodes between containers to make sure
295295
// the containers have compatible allocators.
296296
//
297+
// - `_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN` -- checks that the given argument is within the domain of valid arguments
298+
// for the function. Violating this typically produces an incorrect result (e.g. the clamp algorithm returns the
299+
// original value without clamping it due to incorrect functors) or puts an object into an invalid state (e.g.
300+
// a string view where only a subset of elements is possible to access). This category is for assertions violating
301+
// which doesn't cause any immediate issues in the library -- whatever the consequences are, they will happen in the
302+
// user code.
303+
//
297304
// - `_LIBCPP_ASSERT_PEDANTIC` -- checks prerequisites which are imposed by the Standard, but violating which happens to
298305
// be benign in our implementation.
299306
//
@@ -338,6 +345,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
338345
// Overlapping ranges will make algorithms produce incorrect results but don't directly lead to a security
339346
// vulnerability.
340347
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSUME(expression)
348+
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression)
341349
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression)
342350
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
343351
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
@@ -353,8 +361,9 @@ _LIBCPP_HARDENING_MODE_DEBUG
353361
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSERT(expression, message)
354362
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSERT(expression, message)
355363
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message)
356-
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
364+
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSERT(expression, message)
357365
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
366+
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
358367
// Disabled checks.
359368
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
360369

@@ -368,6 +377,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
368377
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSERT(expression, message)
369378
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSERT(expression, message)
370379
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message)
380+
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSERT(expression, message)
371381
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
372382
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSERT(expression, message)
373383
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
@@ -382,6 +392,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
382392
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSUME(expression)
383393
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSUME(expression)
384394
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression)
395+
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression)
385396
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
386397
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
387398
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)

libcxx/include/__hash_table

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ public:
861861

862862
template <class _Key>
863863
_LIBCPP_HIDE_FROM_ABI size_type bucket(const _Key& __k) const {
864-
_LIBCPP_ASSERT_UNCATEGORIZED(
864+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
865865
bucket_count() > 0, "unordered container::bucket(key) called when bucket_count() == 0");
866866
return std::__constrain_hash(hash_function()(__k), bucket_count());
867867
}

libcxx/include/__memory/assume_aligned.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp* __ass
2929
if (__libcpp_is_constant_evaluated()) {
3030
return __ptr;
3131
} else {
32-
_LIBCPP_ASSERT_UNCATEGORIZED(reinterpret_cast<uintptr_t>(__ptr) % _Np == 0, "Alignment assumption is violated");
32+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
33+
reinterpret_cast<uintptr_t>(__ptr) % _Np == 0, "Alignment assumption is violated");
3334
return static_cast<_Tp*>(__builtin_assume_aligned(__ptr, _Np));
3435
}
3536
}

libcxx/include/__numeric/gcd_lcm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI common_type_t<_Tp, _Up> lcm(_Tp __m, _Up
7777
using _Rp = common_type_t<_Tp, _Up>;
7878
_Rp __val1 = __ct_abs<_Rp, _Tp>()(__m) / std::gcd(__m, __n);
7979
_Rp __val2 = __ct_abs<_Rp, _Up>()(__n);
80-
_LIBCPP_ASSERT_UNCATEGORIZED((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm");
80+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm");
8181
return __val1 * __val2;
8282
}
8383

libcxx/include/barrier

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public:
129129
__completion_(std::move(__completion)),
130130
__phase_(0) {}
131131
[[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI arrival_token arrive(ptrdiff_t __update) {
132-
_LIBCPP_ASSERT_UNCATEGORIZED(
132+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
133133
__update <= __expected_, "update is greater than the expected count for the current barrier phase");
134134

135135
auto const __old_phase = __phase_.load(memory_order_relaxed);
@@ -187,7 +187,7 @@ public:
187187
auto const __result = __arrived.fetch_sub(update, memory_order_acq_rel) - update;
188188
auto const new_expected = __expected.load(memory_order_relaxed);
189189

190-
_LIBCPP_ASSERT_UNCATEGORIZED(
190+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
191191
update <= new_expected, "update is greater than the expected count for the current barrier phase");
192192

193193
if (0 == __result) {
@@ -232,7 +232,7 @@ public:
232232
auto const __inc = __arrived_unit * update;
233233
auto const __old = __phase_arrived_expected.fetch_add(__inc, memory_order_acq_rel);
234234

235-
_LIBCPP_ASSERT_UNCATEGORIZED(
235+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
236236
update <= __old, "update is greater than the expected count for the current barrier phase");
237237

238238
if ((__old ^ (__old + __inc)) & __phase_bit) {
@@ -268,10 +268,10 @@ public:
268268
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI explicit barrier(
269269
ptrdiff_t __count, _CompletionF __completion = _CompletionF())
270270
: __b_(__count, std::move(__completion)) {
271-
_LIBCPP_ASSERT_UNCATEGORIZED(
271+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
272272
__count >= 0,
273273
"barrier::barrier(ptrdiff_t, CompletionFunction): barrier cannot be initialized with a negative value");
274-
_LIBCPP_ASSERT_UNCATEGORIZED(
274+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
275275
__count <= max(),
276276
"barrier::barrier(ptrdiff_t, CompletionFunction): barrier cannot be initialized with "
277277
"a value greater than max()");
@@ -281,7 +281,7 @@ public:
281281
barrier& operator=(barrier const&) = delete;
282282

283283
[[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI arrival_token arrive(ptrdiff_t __update = 1) {
284-
_LIBCPP_ASSERT_UNCATEGORIZED(__update > 0, "barrier:arrive must be called with a value greater than 0");
284+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(__update > 0, "barrier:arrive must be called with a value greater than 0");
285285
return __b_.arrive(__update);
286286
}
287287
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void wait(arrival_token&& __phase) const {

libcxx/include/latch

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ public:
7373
static _LIBCPP_HIDE_FROM_ABI constexpr ptrdiff_t max() noexcept { return numeric_limits<ptrdiff_t>::max(); }
7474

7575
inline _LIBCPP_HIDE_FROM_ABI constexpr explicit latch(ptrdiff_t __expected) : __a_(__expected) {
76-
_LIBCPP_ASSERT_UNCATEGORIZED(
76+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
7777
__expected >= 0,
7878
"latch::latch(ptrdiff_t): latch cannot be "
7979
"initialized with a negative value");
80-
_LIBCPP_ASSERT_UNCATEGORIZED(
80+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
8181
__expected <= max(),
8282
"latch::latch(ptrdiff_t): latch cannot be "
8383
"initialized with a value greater than max()");
@@ -88,9 +88,9 @@ public:
8888
latch& operator=(const latch&) = delete;
8989

9090
inline _LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void count_down(ptrdiff_t __update = 1) {
91-
_LIBCPP_ASSERT_UNCATEGORIZED(__update >= 0, "latch::count_down called with a negative value");
91+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(__update >= 0, "latch::count_down called with a negative value");
9292
auto const __old = __a_.fetch_sub(__update, memory_order_release);
93-
_LIBCPP_ASSERT_UNCATEGORIZED(
93+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
9494
__update <= __old,
9595
"latch::count_down called with a value greater "
9696
"than the internal counter");
@@ -102,7 +102,7 @@ public:
102102
__cxx_atomic_wait(&__a_.__a_, [this]() -> bool { return try_wait(); });
103103
}
104104
inline _LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void arrive_and_wait(ptrdiff_t __update = 1) {
105-
_LIBCPP_ASSERT_UNCATEGORIZED(__update >= 0, "latch::arrive_and_wait called with a negative value");
105+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(__update >= 0, "latch::arrive_and_wait called with a negative value");
106106
// other preconditions on __update are checked in count_down()
107107

108108
count_down(__update);

libcxx/include/semaphore

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public:
9292
_LIBCPP_HIDE_FROM_ABI constexpr explicit __atomic_semaphore_base(ptrdiff_t __count) : __a_(__count) {}
9393
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void release(ptrdiff_t __update = 1) {
9494
auto __old = __a_.fetch_add(__update, memory_order_release);
95-
_LIBCPP_ASSERT_UNCATEGORIZED(
95+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
9696
__update <= _LIBCPP_SEMAPHORE_MAX - __old, "update is greater than the expected value");
9797

9898
if (__old > 0) {
@@ -138,11 +138,11 @@ public:
138138
static constexpr ptrdiff_t max() noexcept { return __least_max_value; }
139139

140140
_LIBCPP_HIDE_FROM_ABI constexpr explicit counting_semaphore(ptrdiff_t __count) : __semaphore_(__count) {
141-
_LIBCPP_ASSERT_UNCATEGORIZED(
141+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
142142
__count >= 0,
143143
"counting_semaphore::counting_semaphore(ptrdiff_t): counting_semaphore cannot be "
144144
"initialized with a negative value");
145-
_LIBCPP_ASSERT_UNCATEGORIZED(
145+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
146146
__count <= max(),
147147
"counting_semaphore::counting_semaphore(ptrdiff_t): counting_semaphore cannot be "
148148
"initialized with a value greater than max()");
@@ -153,7 +153,7 @@ public:
153153
counting_semaphore& operator=(const counting_semaphore&) = delete;
154154

155155
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void release(ptrdiff_t __update = 1) {
156-
_LIBCPP_ASSERT_UNCATEGORIZED(__update >= 0, "counting_semaphore:release called with a negative value");
156+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(__update >= 0, "counting_semaphore:release called with a negative value");
157157
__semaphore_.release(__update);
158158
}
159159
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void acquire() { __semaphore_.acquire(); }

libcxx/include/string_view

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,11 @@ public:
307307
: __data_(__s),
308308
__size_(__len) {
309309
#if _LIBCPP_STD_VER >= 14
310-
_LIBCPP_ASSERT_UNCATEGORIZED(__len <= static_cast<size_type>(numeric_limits<difference_type>::max()),
311-
"string_view::string_view(_CharT *, size_t): length does not fit in difference_type");
310+
// This will result in creating an invalid `string_view` object -- some calculations involving `size` would
311+
// overflow, making it effectively truncated.
312+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
313+
__len <= static_cast<size_type>(numeric_limits<difference_type>::max()),
314+
"string_view::string_view(_CharT *, size_t): length does not fit in difference_type");
312315
_LIBCPP_ASSERT_NON_NULL(
313316
__len == 0 || __s != nullptr, "string_view::string_view(_CharT *, size_t): received nullptr");
314317
#endif

libcxx/src/filesystem/operations.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,9 @@ void __permissions(const path& p, perms prms, perm_options opts, error_code* ec)
608608
const bool resolve_symlinks = !has_opt(perm_options::nofollow);
609609
const bool add_perms = has_opt(perm_options::add);
610610
const bool remove_perms = has_opt(perm_options::remove);
611-
_LIBCPP_ASSERT_UNCATEGORIZED(
611+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
612612
(add_perms + remove_perms + has_opt(perm_options::replace)) == 1,
613-
"One and only one of the perm_options constants replace, add, or remove "
614-
"is present in opts");
613+
"One and only one of the perm_options constants 'replace', 'add', or 'remove' must be present in opts");
615614

616615
bool set_sym_perms = false;
617616
prms &= perms::mask;

libcxx/src/include/to_chars_floating_point.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,7 @@ to_chars_result _Floating_to_chars(
994994
if constexpr (_Overload == _Floating_to_chars_overload::_Plain) {
995995
_LIBCPP_ASSERT_INTERNAL(_Fmt == chars_format{}, ""); // plain overload must pass chars_format{} internally
996996
} else {
997-
_LIBCPP_ASSERT_UNCATEGORIZED(_Fmt == chars_format::general || _Fmt == chars_format::scientific
997+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(_Fmt == chars_format::general || _Fmt == chars_format::scientific
998998
|| _Fmt == chars_format::fixed || _Fmt == chars_format::hex,
999999
"invalid format in to_chars()");
10001000
}

0 commit comments

Comments
 (0)