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

Improve debug checks (prelude to STL Hardening) #5270

Merged
merged 35 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
0962eb1
Remove CDL preprocessor comments after a single line.
StephanTLavavej Jan 27, 2025
acc4f34
Remove CDL preprocessor comments after a single `_STL_VERIFY` over mu…
StephanTLavavej Jan 27, 2025
c20b369
docs/import_library.md: Don't mention CDL - it doesn't affect ABI, an…
StephanTLavavej Jan 27, 2025
2cb3a35
Improve debug messages: "array<T, 0> subscript invalid"
StephanTLavavej Jan 31, 2025
06a1361
Improve debug messages: "MEOW subscript out of range"
StephanTLavavej Jan 31, 2025
97d2acb
Improve debug messages: "MEOW() called on empty WOOF"
StephanTLavavej Jan 31, 2025
a7ce525
Improve debug messages: Clarify span's first<Count>(), last<Count>(),…
StephanTLavavej Jan 31, 2025
f325132
Improve debug messages: Clarify string_view remove_prefix(), remove_s…
StephanTLavavej Jan 31, 2025
a3de284
Improve debug messages: std::expected.
StephanTLavavej Jan 31, 2025
a5ab1f5
Improve debug messages: Grammar.
StephanTLavavej Feb 4, 2025
eca9c57
Test cleanup: Remove CDL definitions from some `<ranges>` tests.
StephanTLavavej Jan 28, 2025
fd0c6f1
Test cleanup: Drop CDL from `<format>` test, properly test negative d…
StephanTLavavej Jan 28, 2025
2007c12
Test cleanup: Remove VSO-847348 workarounds.
StephanTLavavej Feb 5, 2025
ed90fb1
Test cleanup: Use `if constexpr` instead of tag dispatch and plain `if`.
StephanTLavavej Feb 5, 2025
6554e97
Code cleanups: Add const, unwrap "strengthened".
StephanTLavavej Feb 5, 2025
a78ff8d
Code cleanup: Conditional `_STL_REPORT_ERROR` => `_STL_VERIFY` or `_S…
StephanTLavavej Feb 1, 2025
88a59ba
Code cleanup: `_STL_VERIFY(false)` => `_STL_REPORT_ERROR`
StephanTLavavej Feb 1, 2025
71dcd4f
Code cleanup: Consistently order `case` before `default`.
StephanTLavavej Feb 4, 2025
25960e5
Code cleanup: Fuse _Check_alignment into atomic_ref ctor.
StephanTLavavej Feb 4, 2025
85f29dc
Code cleanup: Collapse _ATOMIC_REF_CHECK_ALIGNMENT into _STL_ASSERT.
StephanTLavavej Feb 4, 2025
57e3cca
Code cleanup: Avoid duplicating _Off and _Count. Add const to _Moved.
StephanTLavavej Feb 4, 2025
22a4cbb
Code cleanup: Avoid duplicating _STD copy() call.
StephanTLavavej Feb 4, 2025
59512b9
Code cleanup: Avoid more duplication.
StephanTLavavej Feb 4, 2025
62ce28d
Code cleanup: IDL == 2 implies debug, so _STL_ASSERT should be _STL_V…
StephanTLavavej Feb 4, 2025
8c69d19
Code cleanup: Extract _Can_memcpy for clarity, simplifying following …
StephanTLavavej Feb 4, 2025
451c7ba
Enhancement (for CoE): Adjust uninitialized_meow's logic.
StephanTLavavej Feb 4, 2025
33df586
Enhancement (for CoE): After _INVALID_MEMORY_ORDER, consistently fall…
StephanTLavavej Feb 4, 2025
81776a0
Enhancement: _STL_ASSERT => _STL_VERIFY for checks within IDL >= 1.
StephanTLavavej Feb 4, 2025
0f562b8
Enhancement: Replace _STL_ASSERT with _STL_INTERNAL_STATIC_ASSERT.
StephanTLavavej Feb 4, 2025
925cd6a
Enhancement: Remove IDL=2 null asserts from auto_ptr deref/arrow.
StephanTLavavej Feb 4, 2025
a6ea00a
Enhancement: _Span_iterator::operator-= was unconditionally checking.
StephanTLavavej Feb 4, 2025
b1f76b8
Enhancement: Improve `_STL_REPORT_ERROR`, `_STL_VERIFY` codegen.
StephanTLavavej Feb 5, 2025
05fdc63
Code cleanup: Use `_Check_MEOW_memory_order` to avoid some `_FALLTHRO…
StephanTLavavej Feb 5, 2025
7f6b11b
Improve debug messages: "stores" => "contains"
StephanTLavavej Feb 6, 2025
5fee29e
Code review feedback: Improve debug messages and whitespace.
StephanTLavavej Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/import_library.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The caveats of this technique are:
+ This limitation is subtle (not readily apparent from the source code) and critical.
If shared global state is necessary, our only option while preserving bincompat is adding a satellite DLL.
* Due to having just two flavors of the import library (debug and release),
we cannot use anything that depends on `_CONTAINER_DEBUG_LEVEL` or `_ITERATOR_DEBUG_LEVEL`.
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
we cannot use anything that depends on `_ITERATOR_DEBUG_LEVEL`.

For these reasons, especially the last one, we need to strictly control what is used by the import library.
In particular, `basic_string` must not be used there.
Expand Down
2 changes: 1 addition & 1 deletion stl/inc/__msvc_ranges_tuple_formatter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ class basic_format_arg {
case _Basic_format_arg_type::_Custom_type:
return _STD forward<_Visitor>(_Vis)(_Custom_state);
default:
_STL_VERIFY(false, "basic_format_arg is in impossible state");
_STL_REPORT_ERROR("basic_format_arg is in an impossible state");
int _Dummy{};
return _STD forward<_Visitor>(_Vis)(_Dummy);
}
Expand Down
20 changes: 10 additions & 10 deletions stl/inc/__msvc_string_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ class basic_string_view { // wrapper for any kind of contiguous character buffer
: _Mydata(_Cts), _Mysize(_Count) {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Count == 0 || _Cts, "non-zero size null string_view");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif
}

#if _HAS_CXX20
Expand Down Expand Up @@ -1476,7 +1476,7 @@ class basic_string_view { // wrapper for any kind of contiguous character buffer
_NODISCARD constexpr const_reference operator[](const size_type _Off) const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Off < _Mysize, "string_view subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

// CodeQL [SM01954] This index is optionally validated above.
return _Mydata[_Off];
Expand All @@ -1490,30 +1490,30 @@ class basic_string_view { // wrapper for any kind of contiguous character buffer

_NODISCARD constexpr const_reference front() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize != 0, "cannot call front on empty string_view");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize != 0, "front() called on empty string_view");
#endif
return _Mydata[0];
}

_NODISCARD constexpr const_reference back() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize != 0, "cannot call back on empty string_view");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize != 0, "back() called on empty string_view");
#endif
return _Mydata[_Mysize - 1];
}

constexpr void remove_prefix(const size_type _Count) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize >= _Count, "cannot remove prefix longer than total size");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize >= _Count, "cannot remove_prefix() larger than string_view size");
#endif
_Mydata += _Count;
_Mysize -= _Count;
}

constexpr void remove_suffix(const size_type _Count) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize >= _Count, "cannot remove suffix longer than total size");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Mysize >= _Count, "cannot remove_suffix() larger than string_view size");
#endif
_Mysize -= _Count;
}

Expand Down
4 changes: 1 addition & 3 deletions stl/inc/algorithm
Original file line number Diff line number Diff line change
Expand Up @@ -7157,9 +7157,7 @@ _CONSTEXPR20 void sort_heap(_RanIt _First, _RanIt _Last, _Pr _Pred) { // order h
const auto _ULast = _STD _Get_unwrapped(_Last);
#if _ITERATOR_DEBUG_LEVEL == 2
const auto _Counterexample = _STD _Is_heap_until_unchecked(_UFirst, _ULast, _STD _Pass_fn(_Pred));
if (_Counterexample != _ULast) {
_STL_REPORT_ERROR("invalid heap in sort_heap()");
}
_STL_VERIFY(_Counterexample == _ULast, "invalid heap in sort_heap()");
#endif // _ITERATOR_DEBUG_LEVEL == 2
_STD _Sort_heap_unchecked(_UFirst, _ULast, _STD _Pass_fn(_Pred));
}
Expand Down
20 changes: 10 additions & 10 deletions stl/inc/array
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ public:
_NODISCARD _CONSTEXPR17 reference operator[](_In_range_(<, _Size) size_type _Pos) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Size, "array subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Elems[_Pos];
}
Expand All @@ -542,7 +542,7 @@ public:
/* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Size, "array subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Elems[_Pos];
}
Expand Down Expand Up @@ -708,48 +708,48 @@ public:

_NODISCARD reference operator[](size_type) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0> subscript invalid");
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
#endif

return *data();
}

_NODISCARD const_reference operator[](size_type) const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0> subscript invalid");
#endif

return *data();
}

_NODISCARD reference front() noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0>::front() invalid");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return *data();
}

_NODISCARD const_reference front() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0>::front() invalid");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return *data();
}

_NODISCARD reference back() noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0>::back() invalid");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return *data();
}

_NODISCARD const_reference back() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_REPORT_ERROR("array<T, 0>::back() invalid");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return *data();
}
Expand Down
35 changes: 14 additions & 21 deletions stl/inc/atomic
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,17 @@ extern "C" inline void _Check_memory_order(const unsigned int _Order) noexcept {
case _Atomic_memory_order_relaxed: \
_Result = __iso_volatile_load##_Width(_Ptr); \
break; \
case _Atomic_memory_order_release: \
case _Atomic_memory_order_acq_rel: \
default: \
_INVALID_MEMORY_ORDER; \
_FALLTHROUGH; \
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
case _Atomic_memory_order_consume: \
case _Atomic_memory_order_acquire: \
case _Atomic_memory_order_seq_cst: \
_Result = __LOAD_ACQUIRE_ARM64(_Width, _Ptr); \
_Compiler_barrier(); \
break; \
case _Atomic_memory_order_release: \
case _Atomic_memory_order_acq_rel: \
default: \
_Result = __iso_volatile_load##_Width(_Ptr); \
_INVALID_MEMORY_ORDER; \
break; \
}

#endif // _STD_ATOMIC_USE_ARM64_LDAR_STLR == 1
Expand All @@ -158,15 +157,15 @@ extern "C" inline void _Check_memory_order(const unsigned int _Order) noexcept {
switch (_Order_var) { \
case _Atomic_memory_order_relaxed: \
break; \
case _Atomic_memory_order_consume: \
case _Atomic_memory_order_acquire: \
case _Atomic_memory_order_seq_cst: \
_Compiler_or_memory_barrier(); \
break; \
case _Atomic_memory_order_release: \
case _Atomic_memory_order_acq_rel: \
default: \
_INVALID_MEMORY_ORDER; \
_FALLTHROUGH; \
case _Atomic_memory_order_consume: \
case _Atomic_memory_order_acquire: \
case _Atomic_memory_order_seq_cst: \
_Compiler_or_memory_barrier(); \
break; \
}

Expand All @@ -177,10 +176,10 @@ extern "C" inline void _Check_memory_order(const unsigned int _Order) noexcept {
case _Atomic_memory_order_release: \
__STORE_RELEASE(_Width, _Ptr, _Desired); \
return; \
default: \
case _Atomic_memory_order_consume: \
case _Atomic_memory_order_acquire: \
case _Atomic_memory_order_acq_rel: \
default: \
_INVALID_MEMORY_ORDER; \
_FALLTHROUGH;

Expand Down Expand Up @@ -1180,9 +1179,9 @@ struct _Atomic_storage<_Ty&, 16> { // lock-free using 16-byte intrinsics
case memory_order_acquire:
(void) _INTRIN_ACQUIRE(_InterlockedCompareExchange128)(_Storage_ptr, 0, 0, &_Result._Low);
break;
default:
case memory_order_release:
case memory_order_acq_rel:
default:
_INVALID_MEMORY_ORDER;
_FALLTHROUGH;
case memory_order_seq_cst:
Expand Down Expand Up @@ -2328,7 +2327,8 @@ public:

explicit atomic_ref(_Ty& _Value) noexcept /* strengthened */ : _Base(_Value) {
if constexpr (is_always_lock_free) {
_Check_alignment(_Value);
_STL_ASSERT((reinterpret_cast<uintptr_t>(_STD addressof(_Value)) & (required_alignment - 1)) == 0,
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
"atomic_ref underlying object is not aligned as required_alignment");
} else {
this->_Init_spinlock_for_ref();
}
Expand Down Expand Up @@ -2436,13 +2436,6 @@ public:
{
const_cast<atomic_ref*>(this)->_Base::notify_all();
}

private:
static void _Check_alignment([[maybe_unused]] const _Ty& _Value) {
_ATOMIC_REF_CHECK_ALIGNMENT(
(reinterpret_cast<uintptr_t>(_STD addressof(_Value)) & (required_alignment - 1)) == 0,
"atomic_ref underlying object is not aligned as required_alignment");
}
};
#endif // _HAS_CXX20

Expand Down
4 changes: 2 additions & 2 deletions stl/inc/bitset
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private:
#if _ITERATOR_DEBUG_LEVEL == 0
(void) _Pos;
#else // ^^^ _ITERATOR_DEBUG_LEVEL == 0 / _ITERATOR_DEBUG_LEVEL != 0 vvv
_STL_VERIFY(_Pos < _Bits, "bitset index outside range");
_STL_VERIFY(_Pos < _Bits, "bitset subscript out of range");
#endif // ^^^ _ITERATOR_DEBUG_LEVEL != 0 ^^^
}

Expand All @@ -132,7 +132,7 @@ private:
static constexpr unsigned long long _Mask = (1ULL << (_Need_mask ? _Bits : 0)) - 1ULL;

public:
_NODISCARD constexpr bool operator[](size_t _Pos) const noexcept /* strengthened */ {
_NODISCARD constexpr bool operator[](const size_t _Pos) const noexcept /* strengthened */ {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_Validate(_Pos);
return _Subscript(_Pos);
}
Expand Down
6 changes: 5 additions & 1 deletion stl/inc/chrono
Original file line number Diff line number Diff line change
Expand Up @@ -1608,8 +1608,12 @@ namespace chrono {
// Returns the number of fractional digits of _Num / _Den in the range [0, 18].
// If it can't be represented, 6 is returned.
// Example: _Fractional_width(1, 8) would return 3 for 0.125.

_STL_INTERNAL_STATIC_ASSERT(_Duration::period::num > 0); // N5001 [time.duration.general]/3
_STL_INTERNAL_STATIC_ASSERT(_Duration::period::den > 0); // N5001 [ratio.ratio]/1, /2.2

auto _Den = _Duration::period::den;
_STL_ASSERT(_Duration::period::num > 0 && _Den > 0, "Numerator and denominator can't be less than 1.");

StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
unsigned int _Power_of_2_in_den = 0u;
unsigned int _Power_of_5_in_den = 0u;
for (; _Den % 2 == 0; _Den /= 2, ++_Power_of_2_in_den) {
Expand Down
26 changes: 13 additions & 13 deletions stl/inc/deque
Original file line number Diff line number Diff line change
Expand Up @@ -1065,15 +1065,15 @@ public:
_NODISCARD const_reference operator[](size_type _Pos) const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(_Pos);
}

_NODISCARD reference operator[](size_type _Pos) noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(_Pos);
}
Expand All @@ -1097,31 +1097,31 @@ public:
_NODISCARD reference front() noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(!empty(), "front() called on empty deque");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(0);
}

_NODISCARD const_reference front() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(!empty(), "front() called on empty deque");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(0);
}

_NODISCARD reference back() noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(!empty(), "back() called on empty deque");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(_Mysize() - 1);
}

_NODISCARD const_reference back() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(!empty(), "back() called on empty deque");
#endif // _CONTAINER_DEBUG_LEVEL > 0
#endif

return _Subscript(_Mysize() - 1);
}
Expand Down Expand Up @@ -1475,7 +1475,7 @@ public:
void pop_front() noexcept /* strengthened */ {
#if _ITERATOR_DEBUG_LEVEL == 2
if (empty()) {
_STL_REPORT_ERROR("deque empty before pop");
_STL_REPORT_ERROR("pop_front() called on empty deque");
} else { // something to erase, do it
_Orphan_off(_Myoff());
_Alty_traits::destroy(_Getal(), _Get_data()._Address_subscript(_Myoff()));
Expand All @@ -1498,7 +1498,7 @@ public:
void pop_back() noexcept /* strengthened */ {
#if _ITERATOR_DEBUG_LEVEL == 2
if (empty()) {
_STL_REPORT_ERROR("deque empty before pop");
_STL_REPORT_ERROR("pop_back() called on empty deque");
} else { // something to erase, do it
size_type _Newoff = _Myoff() + _Mysize() - 1;
_Orphan_off(_Newoff);
Expand Down Expand Up @@ -1528,14 +1528,14 @@ public:
#if _ITERATOR_DEBUG_LEVEL == 2
_STL_VERIFY(_First <= _Last && begin() <= _First && _Last <= end(), "deque erase iterator outside range");
_STD _Adl_verify_range(_First, _Last);
#endif

auto _Off = static_cast<size_type>(_First - begin());
auto _Count = static_cast<size_type>(_Last - _First);
bool _Moved = _Off > 0 && _Off + _Count < _Mysize();
#else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv
auto _Off = static_cast<size_type>(_First - begin());
auto _Count = static_cast<size_type>(_Last - _First);
#endif // ^^^ _ITERATOR_DEBUG_LEVEL < 2 ^^^

#if _ITERATOR_DEBUG_LEVEL == 2
const bool _Moved = _Off > 0 && _Off + _Count < _Mysize();
#endif

if (_Count == 0) {
return _First;
Expand Down
Loading