Message ID | 653c3e3c-14e8-af57-c9b8-75f82950fc87@gmail.com |
---|---|
State | New |
Headers | show |
Series | Enable string_view assertions | expand |
(Resending from a different account, sorry for the duplicate). On 14/03/18 22:12 +0100, François Dumont wrote: > constexpr const _CharT& > operator[](size_type __pos) const noexcept > { >- // TODO: Assert to restore in a way compatible with the constexpr. >- // __glibcxx_assert(__pos < this->_M_len); >+ if (!__builtin_constant_p(__pos)) >+ __glibcxx_assert(__pos < this->_M_len); This doesn't do the right thing, because it fails to assert when __pos is known at compile-time: const std::string_view sv; sv[100]; This will only assert at -O0. As soon as optimization is enabled the value is known inside the function, and __builtin_constant_p(__pos) is true, and we don't do the range check. We also don't do the check for constant expressions, when we should be able to give a compilation error, not just ignore it! Enabling the assertions at -O0 and outside constant expressions is slightly better than never enabling them at all, but not good enough to remove the "TODO" comments. Ideally we want out-of-range accesses to be an error in constant expressions, and fail the runtime assertion in non-constant expressions. But using __builtin_constant_p to do the assertion conditionally doesn't do that. In PR 78420 either branch is OK: when the comparison is known at compile-time, do it directly, otherwise do it via casting. Both give the same result. Here you do "if the result is not known, check it at runtime, otherwise don't check at all". What we should do instead is declare a new member function like this: static void __out_of_range() __attribute__((__error__("Index out-of-range"))); Then we can refer to that in the cases where (__pos >= _M_len) is known at compile-time: constexpr const _CharT& operator[](size_type __pos) const noexcept { if (__builtin_constant_p(__pos >= _M_len)) { if (__pos >= _M_len) __out_of_range(); } else __glibcxx_assert(__pos < this->_M_len); return *(this->_M_str + __pos); } Now if we try an out-of-range access in a constant expression we get: sv.cc:4:23: in 'constexpr' expansion of 's.std::basic_string_view<char>::operator[](1)' /home/jwakely/gcc/8/include/c++/8.0.1/string_view:181:22: error: call to non-'constexpr' function 'static void std::basic_string_view<_CharT, _Traits>::__out_of_range() [with _CharT = char; _Traits = std::char_traits<char>]' __out_of_range(); ~~~~~~~~~~~~~~^~ In a non-constant expression, with optimization it gives a compile-time error because of the __error__ attribute: In member function 'constexpr const _CharT& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>]', inlined from 'int main()' at sv.cc:9:6: /home/jwakely/gcc/8/include/c++/8.0.1/string_view:181:22: error: call to 'std::basic_string_view<char>::__out_of_range' declared with attribute error: Index out-of-range __out_of_range(); ~~~~~~~~~~~~~~^~ Finally, without optimization, you can get a run-time failure with _GLIBCXX_ASSERTIONS: /home/jwakely/gcc/8/include/c++/8.0.1/string_view:178: constexpr const _CharT& std::basic_string_view<_CharT, _Traits>::operator[](std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>; std::basic_string_view<_CharT, _Traits>::size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed. Aborted (core dumped) Without _GLIBCXX_ASSERTIONS and without optimization the code compiles and runs, with undefined behaviour. This seems like the optimal set of checks (I've been working on doing this in other types too, so we give compile-time errors when we can prove the code has a bug). >--- /dev/null >+++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2_neg.cc >@@ -0,0 +1,30 @@ >+// { dg-do run { xfail *-*-* } } >+// { dg-options "-std=gnu++17 -O0" } >+// { dg-require-debug-mode "" } Instead of requiring debug mode we could just define _GLIBCXX_ASSERTIONS. We should also add tests for the compile-time errors, both in and out of constant expressions.
On 14 March 2018 at 22:52, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > (Resending from a different account, sorry for the duplicate). > > On 14/03/18 22:12 +0100, François Dumont wrote: >> constexpr const _CharT& >> operator[](size_type __pos) const noexcept >> { >>- // TODO: Assert to restore in a way compatible with the constexpr. >>- // __glibcxx_assert(__pos < this->_M_len); >>+ if (!__builtin_constant_p(__pos)) >>+ __glibcxx_assert(__pos < this->_M_len); > > This doesn't do the right thing, because it fails to assert when __pos > is known at compile-time: > > const std::string_view sv; > sv[100]; > > This will only assert at -O0. As soon as optimization is enabled the > value is known inside the function, and __builtin_constant_p(__pos) > is true, and we don't do the range check. > > We also don't do the check for constant expressions, when we should be > able to give a compilation error, not just ignore it! > > Enabling the assertions at -O0 and outside constant expressions is > slightly better than never enabling them at all, but not good enough > to remove the "TODO" comments. Ideally we want out-of-range accesses > to be an error in constant expressions, and fail the runtime assertion > in non-constant expressions. > > But using __builtin_constant_p to do the assertion conditionally > doesn't do that. In PR 78420 either branch is OK: when the comparison > is known at compile-time, do it directly, otherwise do it via casting. > Both give the same result. > > Here you do "if the result is not known, check it at runtime, > otherwise don't check at all". > > What we should do instead is declare a new member function like this: > > static void > __out_of_range() __attribute__((__error__("Index out-of-range"))); > > Then we can refer to that in the cases where (__pos >= _M_len) is > known at compile-time: > > constexpr const _CharT& > operator[](size_type __pos) const noexcept > { > if (__builtin_constant_p(__pos >= _M_len)) > { > if (__pos >= _M_len) > __out_of_range(); > } > else > __glibcxx_assert(__pos < this->_M_len); > return *(this->_M_str + __pos); > } > > Now if we try an out-of-range access in a constant expression we get: > > sv.cc:4:23: in 'constexpr' expansion of > 's.std::basic_string_view<char>::operator[](1)' > /home/jwakely/gcc/8/include/c++/8.0.1/string_view:181:22: error: call > to non-'constexpr' function 'static void > std::basic_string_view<_CharT, _Traits>::__out_of_range() [with _CharT > = char; _Traits = std::char_traits<char>]' > __out_of_range(); > ~~~~~~~~~~~~~~^~ > > In a non-constant expression, with optimization it gives a > compile-time error because of the __error__ attribute: > > In member function 'constexpr const _CharT& > std::basic_string_view<_CharT, > _Traits>::operator[](std::basic_string_view<_CharT, > _Traits>::size_type) const [with _CharT = char; _Traits = > std::char_traits<char>]', > inlined from 'int main()' at sv.cc:9:6: > /home/jwakely/gcc/8/include/c++/8.0.1/string_view:181:22: error: call > to 'std::basic_string_view<char>::__out_of_range' declared with > attribute error: Index out-of-range > __out_of_range(); > ~~~~~~~~~~~~~~^~ > > Finally, without optimization, you can get a run-time failure with > _GLIBCXX_ASSERTIONS: > > /home/jwakely/gcc/8/include/c++/8.0.1/string_view:178: constexpr const > _CharT& std::basic_string_view<_CharT, > _Traits>::operator[](std::basic_string_view<_CharT, > _Traits>::size_type) const [with _CharT = char; _Traits = > std::char_traits<char>; std::basic_string_view<_CharT, > _Traits>::size_type = long unsigned int]: Assertion '__pos < > this->_M_len' failed. > Aborted (core dumped) > > Without _GLIBCXX_ASSERTIONS and without optimization the code compiles > and runs, with undefined behaviour. > > This seems like the optimal set of checks (I've been working on doing > this in other types too, so we give compile-time errors when we can > prove the code has a bug). Here's one way to generalize this idea. We could potentially replace most of the lightweight __glibcxx_assert checks with this, to get zero-overhead static checking at compile-time whenever possible (even in constexpr functions) and have optional run-time assertions for the remaining cases. diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index 1eb4679f67c..3342af3a4f0 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -462,6 +462,12 @@ namespace std # define __glibcxx_assert(_Condition) #endif +#define __glibcxx_assert2(_Condition, _Action) \ + do { \ + if (__builtin_constant_p((_Condition))) { if (!(_Condition)) _Action; \ + } else { __glibcxx_assert(_Condition); } \ + } while (false) + // Macros for race detectors. // _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(A) and // _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(A) should be used to explain diff --git a/libstdc++-v3/include/std/string_view b/libstdc++-v3/include/std/string_view index 9f39df853e8..a7c74d9d48f 100644 --- a/libstdc++-v3/include/std/string_view +++ b/libstdc++-v3/include/std/string_view @@ -169,8 +169,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr const _CharT& operator[](size_type __pos) const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(__pos < this->_M_len); + __glibcxx_assert2(__pos < this->_M_len, __undefined()); return *(this->_M_str + __pos); } @@ -187,16 +186,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr const _CharT& front() const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + __glibcxx_assert2(this->_M_len > 0, __undefined()); return *this->_M_str; } constexpr const _CharT& back() const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + __glibcxx_assert2(this->_M_len > 0, __undefined()); return *(this->_M_str + this->_M_len - 1); } @@ -209,7 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr void remove_prefix(size_type __n) noexcept { - __glibcxx_assert(this->_M_len >= __n); + __glibcxx_assert2(this->_M_len >= __n, __undefined()); this->_M_str += __n; this->_M_len -= __n; } @@ -404,6 +401,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } private: + static void + __undefined() __attribute__((__error__("undefined behaviour detected"))); static constexpr int _S_compare(size_type __n1, size_type __n2) noexcept
On 14 March 2018 at 23:27, Jonathan Wakely wrote: > Here's one way to generalize this idea. We could potentially replace > most of the lightweight __glibcxx_assert checks with this, to get > zero-overhead static checking at compile-time whenever possible (even > in constexpr functions) and have optional run-time assertions for the > remaining cases. Thinking about this some more, we probably don't want to do this for most __glibcxx_assert uses, because it's probably rare that we can statically detect most errors in something like std::vector::operator[]. I doubt we would catch many bugs that way, as most bugs would involve non-constant indices and vectors that have changed size dynamically at run-time. It *might* be useful in vector::front, vector::back, string::front, deque::front etc. to catch bugs where users do: std::string s; // ... someFunction(&s.front(), s.size()); It seems most valuable in constexpr functions (where we definitely expect constant arguments in many cases) and where run-time arguments will typically be constants, like in the attached patch for atomic objects. diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/include/bits/atomic_base.h index a1fadcd8056..0f6783d257c 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -186,9 +186,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION clear(memory_order __m = memory_order_seq_cst) noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_consume); - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_consume, __invalid_memory_order()); + __glibcxx_assert2(__b != memory_order_acquire, __invalid_memory_order()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid_memory_order()); __atomic_clear (&_M_i, __m); } @@ -197,9 +197,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION clear(memory_order __m = memory_order_seq_cst) volatile noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_consume); - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_consume, __invalid_memory_order()); + __glibcxx_assert2(__b != memory_order_acquire, __invalid_memory_order()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid_memory_order()); __atomic_clear (&_M_i, __m); } @@ -208,6 +208,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static constexpr __atomic_flag_data_type _S_init(bool __i) { return __i ? __GCC_ATOMIC_TEST_AND_SET_TRUEVAL : 0; } + + static void + __invalid_memory_order() + __attribute__((__error__("invalid memory order for atomic_flag::clear"))); }; @@ -367,9 +371,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION store(__int_type __i, memory_order __m = memory_order_seq_cst) noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); - __glibcxx_assert(__b != memory_order_consume); + __glibcxx_assert2(__b != memory_order_acquire, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b != memory_order_consume, __invalid()); __atomic_store_n(&_M_i, __i, __m); } @@ -379,9 +383,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m = memory_order_seq_cst) volatile noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); - __glibcxx_assert(__b != memory_order_consume); + __glibcxx_assert2(__b != memory_order_acquire, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b != memory_order_consume, __invalid()); __atomic_store_n(&_M_i, __i, __m); } @@ -390,8 +394,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION load(memory_order __m = memory_order_seq_cst) const noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_release); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_release, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); return __atomic_load_n(&_M_i, __m); } @@ -400,8 +404,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION load(memory_order __m = memory_order_seq_cst) const volatile noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_release); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_release, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); return __atomic_load_n(&_M_i, __m); } @@ -427,9 +431,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, __m1, __m2); } @@ -441,9 +445,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, __m1, __m2); } @@ -470,9 +474,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 0, __m1, __m2); } @@ -485,9 +489,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 0, __m1, __m2); } @@ -557,6 +561,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION fetch_xor(__int_type __i, memory_order __m = memory_order_seq_cst) volatile noexcept { return __atomic_fetch_xor(&_M_i, __i, __m); } + + private: + static void + __invalid() + __attribute__((__error__("invalid memory order for atomic operation"))); }; @@ -684,9 +693,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); - __glibcxx_assert(__b != memory_order_consume); + __glibcxx_assert2(__b != memory_order_acquire, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b != memory_order_consume, __invalid()); __atomic_store_n(&_M_p, __p, __m); } @@ -696,9 +705,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __m = memory_order_seq_cst) volatile noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_acquire); - __glibcxx_assert(__b != memory_order_acq_rel); - __glibcxx_assert(__b != memory_order_consume); + __glibcxx_assert2(__b != memory_order_acquire, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b != memory_order_consume, __invalid()); __atomic_store_n(&_M_p, __p, __m); } @@ -707,8 +716,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION load(memory_order __m = memory_order_seq_cst) const noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_release); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_release, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); return __atomic_load_n(&_M_p, __m); } @@ -717,8 +726,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION load(memory_order __m = memory_order_seq_cst) const volatile noexcept { memory_order __b = __m & __memory_order_mask; - __glibcxx_assert(__b != memory_order_release); - __glibcxx_assert(__b != memory_order_acq_rel); + __glibcxx_assert2(__b != memory_order_release, __invalid()); + __glibcxx_assert2(__b != memory_order_acq_rel, __invalid()); return __atomic_load_n(&_M_p, __m); } @@ -730,7 +739,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return __atomic_exchange_n(&_M_p, __p, __m); } - _GLIBCXX_ALWAYS_INLINE __pointer_type exchange(__pointer_type __p, memory_order __m = memory_order_seq_cst) volatile noexcept @@ -745,9 +753,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_p, &__p1, __p2, 0, __m1, __m2); } @@ -760,9 +768,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION memory_order __b2 = __m2 & __memory_order_mask; memory_order __b1 = __m1 & __memory_order_mask; - __glibcxx_assert(__b2 != memory_order_release); - __glibcxx_assert(__b2 != memory_order_acq_rel); - __glibcxx_assert(__b2 <= __b1); + __glibcxx_assert2(__b2 != memory_order_release, __invalid()); + __glibcxx_assert2(__b2 != memory_order_acq_rel, __invalid()); + __glibcxx_assert2(__b2 <= __b1, __invalid()); return __atomic_compare_exchange_n(&_M_p, &__p1, __p2, 0, __m1, __m2); } @@ -786,6 +794,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION fetch_sub(ptrdiff_t __d, memory_order __m = memory_order_seq_cst) volatile noexcept { return __atomic_fetch_sub(&_M_p, _M_type_size(__d), __m); } + + private: + static void + __invalid() + __attribute__((__error__("invalid memory order for atomic operation"))); }; // @} group atomics
On 15 March 2018 at 00:18, Jonathan Wakely wrote: > On 14 March 2018 at 23:27, Jonathan Wakely wrote: >> Here's one way to generalize this idea. We could potentially replace >> most of the lightweight __glibcxx_assert checks with this, to get >> zero-overhead static checking at compile-time whenever possible (even >> in constexpr functions) and have optional run-time assertions for the >> remaining cases. > > Thinking about this some more, we probably don't want to do this for > most __glibcxx_assert uses, because it's probably rare that we can > statically detect most errors in something like > std::vector::operator[]. I doubt we would catch many bugs that way, as > most bugs would involve non-constant indices and vectors that have > changed size dynamically at run-time. > > It *might* be useful in vector::front, vector::back, string::front, > deque::front etc. to catch bugs where users do: > > std::string s; > // ... > someFunction(&s.front(), s.size()); > > It seems most valuable in constexpr functions (where we definitely > expect constant arguments in many cases) and where run-time arguments > will typically be constants, like in the attached patch for atomic > objects. Those patches aren't quite right. It's OK to make compilation fail when undefined behaviour is detected in a constant expression, but if we detect it in other contexts (because optimization means all the inputs are known at compile-time) we need to use __attribute((__warning__(""))) not __error__. Those cases are only undefined if the code is executed, so we can only warn and then fail at run-time.
diff --git a/libstdc++-v3/include/experimental/string_view b/libstdc++-v3/include/experimental/string_view index e42d5ac..97f9312 100644 --- a/libstdc++-v3/include/experimental/string_view +++ b/libstdc++-v3/include/experimental/string_view @@ -178,8 +178,9 @@ inline namespace fundamentals_v1 constexpr const _CharT& operator[](size_type __pos) const { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(__pos < this->_M_len); + if (!__builtin_constant_p(__pos)) + __glibcxx_assert(__pos < this->_M_len); + return *(this->_M_str + __pos); } @@ -198,16 +199,18 @@ inline namespace fundamentals_v1 constexpr const _CharT& front() const { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + if (!__builtin_constant_p(this->_M_len)) + __glibcxx_assert(this->_M_len > 0); + return *this->_M_str; } constexpr const _CharT& back() const { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + if (!__builtin_constant_p(this->_M_len)) + __glibcxx_assert(this->_M_len > 0); + return *(this->_M_str + this->_M_len - 1); } diff --git a/libstdc++-v3/include/std/string_view b/libstdc++-v3/include/std/string_view index 9f39df8..22b168b 100644 --- a/libstdc++-v3/include/std/string_view +++ b/libstdc++-v3/include/std/string_view @@ -169,8 +169,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr const _CharT& operator[](size_type __pos) const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(__pos < this->_M_len); + if (!__builtin_constant_p(__pos)) + __glibcxx_assert(__pos < this->_M_len); + return *(this->_M_str + __pos); } @@ -187,16 +188,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr const _CharT& front() const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + if (!__builtin_constant_p(this->_M_len)) + __glibcxx_assert(this->_M_len > 0); + return *this->_M_str; } constexpr const _CharT& back() const noexcept { - // TODO: Assert to restore in a way compatible with the constexpr. - // __glibcxx_assert(this->_M_len > 0); + if (!__builtin_constant_p(this->_M_len)) + __glibcxx_assert(this->_M_len > 0); + return *(this->_M_str + this->_M_len - 1); } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2.cc deleted file mode 100644 index 4ee2b64..0000000 --- a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2.cc +++ /dev/null @@ -1,30 +0,0 @@ -// { dg-do run { xfail *-*-* } } -// { dg-options "-std=gnu++17 -O0" } -// { dg-require-debug-mode "" } - -// Copyright (C) 2013-2018 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -#include <string_view> - -int -main() -{ - typedef std::string_view string_view_type; - string_view_type s; - s[0]; // abort -} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2_neg.cc new file mode 100644 index 0000000..4ee2b64 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/2_neg.cc @@ -0,0 +1,30 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2013-2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> + +int +main() +{ + typedef std::string_view string_view_type; + string_view_type s; + s[0]; // abort +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/back_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/back_neg.cc new file mode 100644 index 0000000..12afb7e --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/back_neg.cc @@ -0,0 +1,37 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> +#include <testsuite_hooks.h> + +void +test01() +{ + std::string_view str; + str.back(); // abort +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/front_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/front_neg.cc new file mode 100644 index 0000000..3db7f05 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/char/front_neg.cc @@ -0,0 +1,37 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> +#include <testsuite_hooks.h> + +void +test01() +{ + std::string_view str; + str.front(); // abort +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2.cc deleted file mode 100644 index 5b7421f..0000000 --- a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2.cc +++ /dev/null @@ -1,32 +0,0 @@ -// { dg-do run { xfail *-*-* } } -// { dg-options "-std=gnu++17 -O0" } -// { dg-require-debug-mode "" } - -// Copyright (C) 2013-2018 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -#include <string_view> - -// libstdc++/21674 -// NB: Should work without any inlining or optimizations (ie. -O0). -int -main() -{ - typedef std::wstring_view string_view_type; - string_view_type s; - s[0]; // abort -} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2_neg.cc new file mode 100644 index 0000000..5b7421f --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/2_neg.cc @@ -0,0 +1,32 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2013-2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> + +// libstdc++/21674 +// NB: Should work without any inlining or optimizations (ie. -O0). +int +main() +{ + typedef std::wstring_view string_view_type; + string_view_type s; + s[0]; // abort +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/back_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/back_neg.cc new file mode 100644 index 0000000..5620ed3 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/back_neg.cc @@ -0,0 +1,37 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> +#include <testsuite_hooks.h> + +void +test01() +{ + std::wstring_view str; + str.back(); // abort +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/front_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/front_neg.cc new file mode 100644 index 0000000..6a63143 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/element_access/wchar_t/front_neg.cc @@ -0,0 +1,37 @@ +// { dg-do run { xfail *-*-* } } +// { dg-options "-std=gnu++17 -O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <string_view> +#include <testsuite_hooks.h> + +void +test01() +{ + std::wstring_view str; + str.front(); // abort +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2.cc b/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2.cc deleted file mode 100644 index bd566e6..0000000 --- a/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2.cc +++ /dev/null @@ -1,30 +0,0 @@ -// { dg-do run { target c++14 xfail *-*-* } } -// { dg-options "-O0" } -// { dg-require-debug-mode "" } - -// Copyright (C) 2013-2018 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -#include <experimental/string_view> - -int -main() -{ - typedef std::experimental::string_view string_view_type; - string_view_type s; - s[0]; // abort -} diff --git a/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2_neg.cc b/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2_neg.cc new file mode 100644 index 0000000..bd566e6 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/string_view/element_access/char/2_neg.cc @@ -0,0 +1,30 @@ +// { dg-do run { target c++14 xfail *-*-* } } +// { dg-options "-O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2013-2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <experimental/string_view> + +int +main() +{ + typedef std::experimental::string_view string_view_type; + string_view_type s; + s[0]; // abort +} diff --git a/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2.cc b/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2.cc deleted file mode 100644 index 6c97c81..0000000 --- a/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2.cc +++ /dev/null @@ -1,32 +0,0 @@ -// { dg-do run { target c++14 xfail *-*-* } } -// { dg-options "-O0" } -// { dg-require-debug-mode "" } - -// Copyright (C) 2013-2018 Free Software Foundation, Inc. -// -// This file is part of the GNU ISO C++ Library. This library is free -// software; you can redistribute it and/or modify it under the -// terms of the GNU General Public License as published by the -// Free Software Foundation; either version 3, or (at your option) -// any later version. - -// This library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License along -// with this library; see the file COPYING3. If not see -// <http://www.gnu.org/licenses/>. - -#include <experimental/string_view> - -// libstdc++/21674 -// NB: Should work without any inlining or optimizations (ie. -O0). -int -main() -{ - typedef std::experimental::wstring_view string_view_type; - string_view_type s; - s[0]; // abort -} diff --git a/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2_neg.cc b/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2_neg.cc new file mode 100644 index 0000000..6c97c81 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/string_view/element_access/wchar_t/2_neg.cc @@ -0,0 +1,32 @@ +// { dg-do run { target c++14 xfail *-*-* } } +// { dg-options "-O0" } +// { dg-require-debug-mode "" } + +// Copyright (C) 2013-2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <experimental/string_view> + +// libstdc++/21674 +// NB: Should work without any inlining or optimizations (ie. -O0). +int +main() +{ + typedef std::experimental::wstring_view string_view_type; + string_view_type s; + s[0]; // abort +}