* [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks @ 2022-08-14 15:32 François Dumont 2022-08-15 20:26 ` François Dumont 2022-08-26 9:31 ` Jonathan Wakely 0 siblings, 2 replies; 5+ messages in thread From: François Dumont @ 2022-08-14 15:32 UTC (permalink / raw) To: libstdc++; +Cc: gcc-patches I think we can add those checks. Note that I wonder if it was needed as in basic_string_view I see usages of __attribute__((__nonnull__)). But running the test I saw no impact even after I try to apply this attribute to the starts_with/ends_with methods themselves. Also note that several checks like the ones I am adding here are XFAILS when using 'make check' because of the segfault rather than on a proper debug checks. Would you prefer to add dg-require-debug-mode to those ? libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks Add simple checks on C string parameters which should not be null. Review null string checks to show: _String != nullptr rather than: _String != 0 libstdc++-v3/ChangeLog: * include/bits/basic_string.h (starts_with, ends_with): Add __glibcxx_check_string. * include/bits/cow_string.h (starts_with, ends_with): Likewise. * include/debug/debug.h: Use nullptr rather than '0' in checks in C++11. * include/debug/string: Likewise. * testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use __gnu_test::string. * testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Use __gnu_test::wstring. * testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: Use __gnu_test::wstring. * testsuite/21_strings/basic_string/operations/starts_with/char.cc: Use __gnu_test::string. * testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc: New test. * testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc: New test. * testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc: New test. * testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc: New test. Tested under linux normal and debug modes. François ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks 2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont @ 2022-08-15 20:26 ` François Dumont 2022-08-26 9:31 ` Jonathan Wakely 1 sibling, 0 replies; 5+ messages in thread From: François Dumont @ 2022-08-15 20:26 UTC (permalink / raw) To: libstdc++; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2273 bytes --] With the patch ! On 14/08/22 17:32, François Dumont wrote: > I think we can add those checks. > > Note that I wonder if it was needed as in basic_string_view I see > usages of __attribute__((__nonnull__)). But running the test I saw no > impact even after I try to apply this attribute to the > starts_with/ends_with methods themselves. > > Also note that several checks like the ones I am adding here are > XFAILS when using 'make check' because of the segfault rather than on > a proper debug checks. Would you prefer to add dg-require-debug-mode > to those ? > > libstdc++: [_GLIBCXX_DEBUG] Add > basic_string::starts_with/ends_with checks > > Add simple checks on C string parameters which should not be null. > > Review null string checks to show: > _String != nullptr > > rather than: > _String != 0 > > libstdc++-v3/ChangeLog: > > * include/bits/basic_string.h (starts_with, ends_with): > Add __glibcxx_check_string. > * include/bits/cow_string.h (starts_with, ends_with): > Likewise. > * include/debug/debug.h: Use nullptr rather than '0' in > checks in C++11. > * include/debug/string: Likewise. > * > testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use > __gnu_test::string. > * > testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Use > __gnu_test::wstring. > * > testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: > Use __gnu_test::wstring. > * > testsuite/21_strings/basic_string/operations/starts_with/char.cc: Use > __gnu_test::string. > * > testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc: > New test. > * > testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc: > New test. > * > testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc: > New test. > * > testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc: > New test. > > Tested under linux normal and debug modes. > > François > > [-- Attachment #2: debug_string_starts_ends_with.patch --] [-- Type: text/x-patch, Size: 14820 bytes --] diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index b04fba95678..d06330e6c48 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -3402,7 +3402,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 constexpr bool starts_with(const _CharT* __x) const noexcept - { return __sv_type(this->data(), this->size()).starts_with(__x); } + { + __glibcxx_requires_string(__x); + return __sv_type(this->data(), this->size()).starts_with(__x); + } constexpr bool ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept @@ -3414,7 +3417,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 constexpr bool ends_with(const _CharT* __x) const noexcept - { return __sv_type(this->data(), this->size()).ends_with(__x); } + { + __glibcxx_requires_string(__x); + return __sv_type(this->data(), this->size()).ends_with(__x); + } #endif // C++20 #if __cplusplus > 202002L diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h index f16e33ac1ef..59b36a1006a 100644 --- a/libstdc++-v3/include/bits/cow_string.h +++ b/libstdc++-v3/include/bits/cow_string.h @@ -3014,7 +3014,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool starts_with(const _CharT* __x) const noexcept - { return __sv_type(this->data(), this->size()).starts_with(__x); } + { + __glibcxx_requires_string(__x); + return __sv_type(this->data(), this->size()).starts_with(__x); + } bool ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept @@ -3026,7 +3029,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool ends_with(const _CharT* __x) const noexcept - { return __sv_type(this->data(), this->size()).ends_with(__x); } + { + __glibcxx_requires_string(__x); + return __sv_type(this->data(), this->size()).ends_with(__x); + } #endif // C++20 #if __cplusplus > 202011L diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 28e250f0c50..f4233760426 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -118,10 +118,17 @@ namespace __gnu_debug __glibcxx_check_heap(_First,_Last) # define __glibcxx_requires_heap_pred(_First,_Last,_Pred) \ __glibcxx_check_heap_pred(_First,_Last,_Pred) -# define __glibcxx_requires_string(_String) \ +# if __cplusplus < 201103L +# define __glibcxx_requires_string(_String) \ _GLIBCXX_DEBUG_PEDASSERT(_String != 0) -# define __glibcxx_requires_string_len(_String,_Len) \ +# define __glibcxx_requires_string_len(_String,_Len) \ _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0) +# else +# define __glibcxx_requires_string(_String) \ + _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr) +# define __glibcxx_requires_string_len(_String,_Len) \ + _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr || _Len == 0) +# endif # define __glibcxx_requires_irreflexive(_First,_Last) \ __glibcxx_check_irreflexive(_First,_Last) # define __glibcxx_requires_irreflexive2(_First,_Last) \ diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string index c32eb41eacd..f02c74431ed 100644 --- a/libstdc++-v3/include/debug/string +++ b/libstdc++-v3/include/debug/string @@ -50,14 +50,25 @@ #endif #ifdef _GLIBCXX_DEBUG_PEDANTIC -# define __glibcxx_check_string(_String) \ +# if __cplusplus < 201103L +# define __glibcxx_check_string(_String) \ _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0, \ __FILE__, __LINE__, \ __PRETTY_FUNCTION__); -# define __glibcxx_check_string_len(_String,_Len) \ +# define __glibcxx_check_string_len(_String,_Len) \ _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0 || _Len == 0, \ __FILE__, __LINE__, \ __PRETTY_FUNCTION__); +# else +# define __glibcxx_check_string(_String) \ + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr, \ + __FILE__, __LINE__, \ + __PRETTY_FUNCTION__); +# define __glibcxx_check_string_len(_String,_Len) \ + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr || _Len == 0, \ + __FILE__, __LINE__, \ + __PRETTY_FUNCTION__); +# endif #else # define __glibcxx_check_string(_String) # define __glibcxx_check_string_len(_String,_Len) @@ -75,8 +86,13 @@ namespace __gnu_debug const char* __function __attribute__((__unused__))) { #ifdef _GLIBCXX_DEBUG_PEDANTIC +# if __cplusplus < 201103L _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0 || __n == 0, __file, __line, __function); +# else + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr || __n == 0, + __file, __line, __function); +# endif #endif return __s; } @@ -90,8 +106,13 @@ namespace __gnu_debug const char* __function __attribute__((__unused__))) { #ifdef _GLIBCXX_DEBUG_PEDANTIC +# if __cplusplus < 201103L _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0, __file, __line, __function); +# else + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr, + __file, __line, __function); +# endif #endif return __s; } @@ -1033,6 +1054,26 @@ namespace __gnu_debug return _Base::compare(__pos1, __n1, __s, __n2); } +#if __cplusplus >= 202002L + using _Base::starts_with; + + __attribute__((__nonnull__)) constexpr bool + starts_with(const _CharT* __x) const noexcept + { + __glibcxx_check_string(__x); + return _Base::starts_with(__x); + } + + using _Base::ends_with; + + constexpr bool + ends_with(const _CharT* __x) const noexcept + { + __glibcxx_check_string(__x); + return _Base::ends_with(__x); + } +#endif + _Base& _M_base() _GLIBCXX_NOEXCEPT { return *this; } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc index cfe18e0186c..3cf85121e36 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc @@ -20,7 +20,7 @@ // basic_string ends_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const char cstr_suf2[] = ".rgb"; const std::string_view sv_suf2(".rgb"); - const std::string s_test("slugs/slimy.jpg"); + const __gnu_test::string s_test("slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.ends_with(cstr_suf); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc new file mode 100644 index 00000000000..7a7b8dd077d --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2022 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/>. +// +// { dg-options "-std=gnu++2a -O0" } +// { dg-do run { target c++2a xfail *-*-* } } + +#define _GLIBCXX_DEBUG_PEDANTIC + +#include <testsuite_string.h> + +void +test01() +{ + const __gnu_test::string s_test("slugs/slimy.jpg"); + s_test.ends_with(nullptr); +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc index d27e8246fb8..70b522ff69e 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc @@ -20,7 +20,7 @@ // basic_string ends_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const wchar_t cstr_suf2[] = L".rgb"; const std::wstring_view sv_suf2(L".rgb"); - const std::wstring s_test(L"slugs/slimy.jpg"); + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.ends_with(cstr_suf); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc new file mode 100644 index 00000000000..a6881bf406b --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2022 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/>. +// +// { dg-options "-std=gnu++2a -O0" } +// { dg-do run { target c++2a xfail *-*-* } } + +#define _GLIBCXX_DEBUG_PEDANTIC + +#include <testsuite_string.h> + +void +test01() +{ + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); + s_test.ends_with(nullptr); +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc index 5c0b3afc0b6..dddf51cee16 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc @@ -20,7 +20,7 @@ // basic_string begins_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const char cstr_dir2[] = "worms/"; const std::string_view sv_dir2("worms/"); - const std::string s_test("slugs/slimy.jpg"); + const __gnu_test::string s_test("slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.starts_with(cstr_dir); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc new file mode 100644 index 00000000000..f357aef2289 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2022 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/>. +// +// { dg-options "-std=gnu++2a -O0" } +// { dg-do run { target c++2a xfail *-*-* } } + +#define _GLIBCXX_DEBUG_PEDANTIC + +#include <testsuite_string.h> + +void +test01() +{ + const __gnu_test::string s_test("slugs/slimy.jpg"); + s_test.starts_with(nullptr); +} + +int +main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc index eda73212ea0..747b23ae5c2 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc @@ -20,7 +20,7 @@ // basic_string begins_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const wchar_t cstr_dir2[] = L"worms/"; const std::wstring_view sv_dir2(L"worms/"); - const std::wstring s_test(L"slugs/slimy.jpg"); + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.starts_with(cstr_dir); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc new file mode 100644 index 00000000000..90065a459b6 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2022 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/>. +// +// { dg-options "-std=gnu++2a -O0" } +// { dg-do run { target c++2a xfail *-*-* } } + +#define _GLIBCXX_DEBUG_PEDANTIC + +#include <testsuite_string.h> + +void +test01() +{ + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); + s_test.starts_with(nullptr); +} + +int +main() +{ + test01(); + return 0; +} ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks 2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont 2022-08-15 20:26 ` François Dumont @ 2022-08-26 9:31 ` Jonathan Wakely 2022-08-31 4:38 ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont 1 sibling, 1 reply; 5+ messages in thread From: Jonathan Wakely @ 2022-08-26 9:31 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc-patches On Sun, 14 Aug 2022 at 16:34, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > I think we can add those checks. > > Note that I wonder if it was needed as in basic_string_view I see usages > of __attribute__((__nonnull__)). But running the test I saw no impact > even after I try to apply this attribute to the starts_with/ends_with > methods themselves. That should cause warnings, and does when I try it. As you say, the relevant string_view constructor already has that anyway: __attribute__((__nonnull__)) constexpr basic_string_view(const _CharT* __str) noexcept And so does string_view::find. The problem is that those only help if the compiler inlines the calls to those functions and so can propagate the null value all the way down to a function with the attribute. Adding the attribute to the relevant starts_with, ends_with and contains functions makes the diagnostics more likely to be emitted without optimization. > > Also note that several checks like the ones I am adding here are XFAILS > when using 'make check' because of the segfault rather than on a proper > debug checks. Would you prefer to add dg-require-debug-mode to those ? > > libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with > checks > > Add simple checks on C string parameters which should not be null. > > Review null string checks to show: > _String != nullptr > > rather than: > _String != 0 I don't really like the extra complexity in the macros, but this does seem like a nice improvement for what users see. We could use __null for C++98, which is a compiler keyword that expands to a null pointer constant, but I'm not sure if that would be clear to all users or not. Maybe 0 is better. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) 2022-08-26 9:31 ` Jonathan Wakely @ 2022-08-31 4:38 ` François Dumont 2022-08-31 9:25 ` Jonathan Wakely 0 siblings, 1 reply; 5+ messages in thread From: François Dumont @ 2022-08-31 4:38 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 3041 bytes --] If I got your point correctly here is this patch again with just the change on '0' becoming 'nullptr' in assertions. libstdc++: [_GLIBCXX_DEBUG] Review nullptr assertion diagnostics Review null string checks to show: _String != nullptr rather than: _String != 0 libstdc++-v3/ChangeLog: * include/debug/debug.h: Use nullptr rather than '0' in checks in post-C++11. * include/debug/string: Likewise. * testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use __gnu_test::string. * testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc: Likewise. * testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Likewise. * testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: Likewise. * testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc: Likewise. * testsuite/21_strings/basic_string/operations/starts_with/char.cc: Likewise.. Ok to commit ? François On 26/08/22 11:31, Jonathan Wakely wrote: > On Sun, 14 Aug 2022 at 16:34, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: >> I think we can add those checks. >> >> Note that I wonder if it was needed as in basic_string_view I see usages >> of __attribute__((__nonnull__)). But running the test I saw no impact >> even after I try to apply this attribute to the starts_with/ends_with >> methods themselves. > That should cause warnings, and does when I try it. > > As you say, the relevant string_view constructor already has that anyway: > > __attribute__((__nonnull__)) constexpr > basic_string_view(const _CharT* __str) noexcept > > And so does string_view::find. The problem is that those only help if > the compiler inlines the calls to those functions and so can propagate > the null value all the way down to a function with the attribute. > Adding the attribute to the relevant starts_with, ends_with and > contains functions makes the diagnostics more likely to be emitted > without optimization. > >> Also note that several checks like the ones I am adding here are XFAILS >> when using 'make check' because of the segfault rather than on a proper >> debug checks. Would you prefer to add dg-require-debug-mode to those ? >> >> libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with >> checks >> >> Add simple checks on C string parameters which should not be null. >> >> Review null string checks to show: >> _String != nullptr >> >> rather than: >> _String != 0 > I don't really like the extra complexity in the macros, but this does > seem like a nice improvement for what users see. > > We could use __null for C++98, which is a compiler keyword that > expands to a null pointer constant, but I'm not sure if that would be > clear to all users or not. Maybe 0 is better. > > . [-- Attachment #2: debug_nullptr_string.patch --] [-- Type: text/x-patch, Size: 7977 bytes --] diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 28e250f0c50..f4233760426 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -118,10 +118,17 @@ namespace __gnu_debug __glibcxx_check_heap(_First,_Last) # define __glibcxx_requires_heap_pred(_First,_Last,_Pred) \ __glibcxx_check_heap_pred(_First,_Last,_Pred) -# define __glibcxx_requires_string(_String) \ +# if __cplusplus < 201103L +# define __glibcxx_requires_string(_String) \ _GLIBCXX_DEBUG_PEDASSERT(_String != 0) -# define __glibcxx_requires_string_len(_String,_Len) \ +# define __glibcxx_requires_string_len(_String,_Len) \ _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0) +# else +# define __glibcxx_requires_string(_String) \ + _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr) +# define __glibcxx_requires_string_len(_String,_Len) \ + _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr || _Len == 0) +# endif # define __glibcxx_requires_irreflexive(_First,_Last) \ __glibcxx_check_irreflexive(_First,_Last) # define __glibcxx_requires_irreflexive2(_First,_Last) \ diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string index c32eb41eacd..574a78e3dac 100644 --- a/libstdc++-v3/include/debug/string +++ b/libstdc++-v3/include/debug/string @@ -50,14 +50,25 @@ #endif #ifdef _GLIBCXX_DEBUG_PEDANTIC -# define __glibcxx_check_string(_String) \ +# if __cplusplus < 201103L +# define __glibcxx_check_string(_String) \ _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0, \ __FILE__, __LINE__, \ __PRETTY_FUNCTION__); -# define __glibcxx_check_string_len(_String,_Len) \ +# define __glibcxx_check_string_len(_String,_Len) \ _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0 || _Len == 0, \ __FILE__, __LINE__, \ __PRETTY_FUNCTION__); +# else +# define __glibcxx_check_string(_String) \ + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr, \ + __FILE__, __LINE__, \ + __PRETTY_FUNCTION__); +# define __glibcxx_check_string_len(_String,_Len) \ + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr || _Len == 0, \ + __FILE__, __LINE__, \ + __PRETTY_FUNCTION__); +# endif #else # define __glibcxx_check_string(_String) # define __glibcxx_check_string_len(_String,_Len) @@ -75,8 +86,13 @@ namespace __gnu_debug const char* __function __attribute__((__unused__))) { #ifdef _GLIBCXX_DEBUG_PEDANTIC +# if __cplusplus < 201103L _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0 || __n == 0, __file, __line, __function); +# else + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr || __n == 0, + __file, __line, __function); +# endif #endif return __s; } @@ -90,8 +106,13 @@ namespace __gnu_debug const char* __function __attribute__((__unused__))) { #ifdef _GLIBCXX_DEBUG_PEDANTIC +# if __cplusplus < 201103L _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0, __file, __line, __function); +# else + _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr, + __file, __line, __function); +# endif #endif return __s; } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc index cfe18e0186c..3cf85121e36 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc @@ -20,7 +20,7 @@ // basic_string ends_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const char cstr_suf2[] = ".rgb"; const std::string_view sv_suf2(".rgb"); - const std::string s_test("slugs/slimy.jpg"); + const __gnu_test::string s_test("slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.ends_with(cstr_suf); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc index 1f2a156bace..70ab867fa4e 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc @@ -1,10 +1,10 @@ // { dg-options "-std=gnu++20 -Wnonnull -O0" } // { dg-do compile { target c++20 } } -#include <string> +#include <testsuite_string.h> void -test01(const std::string& s) +test01(const __gnu_test::string& s) { s.ends_with((const char*)nullptr); // { dg-warning "\\\[-Wnonnull" } s.ends_with((char*)nullptr); // { dg-warning "\\\[-Wnonnull" } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc index d27e8246fb8..70b522ff69e 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc @@ -20,7 +20,7 @@ // basic_string ends_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const wchar_t cstr_suf2[] = L".rgb"; const std::wstring_view sv_suf2(L".rgb"); - const std::wstring s_test(L"slugs/slimy.jpg"); + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.ends_with(cstr_suf); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc index 5c0b3afc0b6..dddf51cee16 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc @@ -20,7 +20,7 @@ // basic_string begins_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const char cstr_dir2[] = "worms/"; const std::string_view sv_dir2("worms/"); - const std::string s_test("slugs/slimy.jpg"); + const __gnu_test::string s_test("slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.starts_with(cstr_dir); VERIFY( cstr_in_slugs ); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc index 8514359c877..34614be0ab6 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc @@ -1,10 +1,10 @@ // { dg-options "-std=gnu++20 -Wnonnull -O0" } // { dg-do compile { target c++20 } } -#include <string> +#include <testsuite_string.h> void -test01(const std::string& s) +test01(const __gnu_test::string& s) { s.starts_with((const char*)nullptr); // { dg-warning "\\\[-Wnonnull" } s.starts_with((char*)nullptr); // { dg-warning "\\\[-Wnonnull" } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc index eda73212ea0..747b23ae5c2 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc @@ -20,7 +20,7 @@ // basic_string begins_with -#include <string> +#include <testsuite_string.h> #include <testsuite_hooks.h> void @@ -31,7 +31,7 @@ test01() const wchar_t cstr_dir2[] = L"worms/"; const std::wstring_view sv_dir2(L"worms/"); - const std::wstring s_test(L"slugs/slimy.jpg"); + const __gnu_test::wstring s_test(L"slugs/slimy.jpg"); const auto cstr_in_slugs = s_test.starts_with(cstr_dir); VERIFY( cstr_in_slugs ); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) 2022-08-31 4:38 ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont @ 2022-08-31 9:25 ` Jonathan Wakely 0 siblings, 0 replies; 5+ messages in thread From: Jonathan Wakely @ 2022-08-31 9:25 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc-patches On Wed, 31 Aug 2022 at 05:38, François Dumont <frs.dumont@gmail.com> wrote: > > If I got your point correctly here is this patch again with just the > change on '0' becoming 'nullptr' in assertions. > > libstdc++: [_GLIBCXX_DEBUG] Review nullptr assertion diagnostics > > Review null string checks to show: > _String != nullptr > > rather than: > _String != 0 > > libstdc++-v3/ChangeLog: > > * include/debug/debug.h: Use nullptr rather than '0' in > checks in post-C++11. > * include/debug/string: Likewise. > * > testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use > __gnu_test::string. > * > testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc: Likewise. > * > testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Likewise. > * > testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: > Likewise. > * > testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc: > Likewise. > * > testsuite/21_strings/basic_string/operations/starts_with/char.cc: Likewise.. > > Ok to commit ? The testsuite changes seem unrelated to the actual code change, so I'd have done a separate patch. But OK for trunk. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-31 9:25 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont 2022-08-15 20:26 ` François Dumont 2022-08-26 9:31 ` Jonathan Wakely 2022-08-31 4:38 ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont 2022-08-31 9:25 ` Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).