Re: [2/2] Add AddressSanitizer annotations to std::string.
Rebased and update patch (typos, add missing annotations), add ASan teststo verify string annotation. On 06/28/2018 11:09 AM, Mikhail Kashkarov wrote: > ^ gentle ping. > > > On 06/08/2018 05:54 PM, Mikhail Kashkarov wrote: >> Hello, >> >> I've updated patches for std::string sanitization and disabling CXX11 >> string SSO usage for correct sanitization. >> >>>> _M_destroy(_M_allocated_capacity); >>>>+ else >>>>+ __annotate_delete(); >>> >>>Do these calls definitely optimize away completely when not >>>sanitizing? Even for -O1, -Os and -Og? >>> >>>For std::vector annotation I used macros to add these annotations, so >>>there is no change to the generated code when annotations are >>>disabled. But it makes the code quite ugly. >> >> I've checked asm code for string-inst.o and it looks like this calls are >> optimized away, but there are some light changes after patch fir . >> >>> Right, I was wondering specifically about the >>> instantiations. I could be wrong but I don't think anything in >>> creates, destroys or modifies a std::basic_string. >> >> I was confused by reinterpret_cast's on strings in fstream.tcc, looks >> like this is not needed, you are right. >> >>>> // calling 4.0.x+ _S_create. >>>> __p->_M_set_sharable(); >>>>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING >>>>+ __p->_M_length = 0; >>>>+#endif >>> >>> Why is this needed? I think a comment explaining it would help (like >>> the one above explaining why calling _M_set_sharable() is needed). >> >> Checked current version without this change, looks like now it works, >> reverted. >> >> Short summary: >> The reason for changing strings layout under sanitization is to place string >> char buffer on heap for correct aligning in 32-bit environments, >> both pre-CXX11 and CXX11 strings ABI. >> >> | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit | >> |-+-+-++| >> | FULL | SSO-string | yes | + | + | >> | | COW-string | yes | + | + | >> |-+-+-++| >> | PARTIAL | SSO-string | no | -+(*) | + | >> | | COW-string | no | - | + | >> *only longs strings are sanitized for 32-bit >> >> Some functions with new define looks a bit messy without changing internal >> functions(operator=), I'm also not sure if disabling string SSO usage define >> should also affects other parts that relies on basic_string class size >> (checks >> with static_assert in exceptions/shim-facets). >> >> >> Any thoughts? >> >> On 05/29/2018 06:55 PM, Jonathan Wakely wrote: >>> On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote: >>>> Jonathan Wakely writes: >>>>>> --- a/libstdc++-v3/include/bits/fstream.tcc >>>>>> +++ b/libstdc++-v3/include/bits/fstream.tcc >>>>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>> >>>>>> // Inhibit implicit instantiations for required instantiations, >>>>>> // which are defined via explicit instantiations elsewhere. >>>>>> +#if !_GLIBCXX_SANITIZE_STRING >>>>>> #if _GLIBCXX_EXTERN_TEMPLATE >>>>>> extern template class basic_filebuf; >>>>>> extern template class basic_ifstream; >>>>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>> extern template class basic_fstream; >>>>>> #endif >>>>>> #endif >>>>>> +#endif // !_GLIBCXX_SANITIZE_STRING >>>>> Why do we need to disable these explicit instantiation declarations? >>>>> Are they affected by the std::string layout changes? Is that just >>>>> because of the constructors taking std::string, or something else? >>>> Libstdc++ build is not sanitized, so macroses that requires >>>> AddressSanitizer support will not applied and these templates will be >>>> instantate without support for ASan annotations. >>> Right, I was wondering specifically about the >>> instantiations. I could be wrong but I don't t
Re: [2/2] Add AddressSanitizer annotations to std::string.
^^ gentle ping. On 07/16/2018 07:16 PM, Mikhail Kashkarov wrote: > Rebased and update patch (typos, add missing annotations), > add ASan teststo verify string annotation. > > > On 06/28/2018 11:09 AM, Mikhail Kashkarov wrote: >> ^ gentle ping. >> >> >> On 06/08/2018 05:54 PM, Mikhail Kashkarov wrote: >>> Hello, >>> >>> I've updated patches for std::string sanitization and disabling CXX11 >>> string SSO usage for correct sanitization. >>> >>> >> _M_destroy(_M_allocated_capacity); >>> >>+ else >>> >>+ __annotate_delete(); >>> > >>> >Do these calls definitely optimize away completely when not >>> >sanitizing? Even for -O1, -Os and -Og? >>> > >>> >For std::vector annotation I used macros to add these >>> annotations, so >>> >there is no change to the generated code when annotations are >>> >disabled. But it makes the code quite ugly. >>> >>> I've checked asm code for string-inst.o and it looks like this calls >>> are >>> optimized away, but there are some light changes after patch fir . >>> >>> > Right, I was wondering specifically about the >>> > instantiations. I could be wrong but I don't think anything in >>> > creates, destroys or modifies a std::basic_string. >>> >>> I was confused by reinterpret_cast's on strings in fstream.tcc, looks >>> like this is not needed, you are right. >>> >>> >> // calling 4.0.x+ _S_create. >>> >> __p->_M_set_sharable(); >>> >>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING >>> >>+ __p->_M_length = 0; >>> >>+#endif >>> > >>> > Why is this needed? I think a comment explaining it would help >>> (like >>> > the one above explaining why calling _M_set_sharable() is needed). >>> >>> Checked current version without this change, looks like now it works, >>> reverted. >>> >>> Short summary: >>> The reason for changing strings layout under sanitization is to >>> place string >>> char buffer on heap for correct aligning in 32-bit environments, >>> both pre-CXX11 and CXX11 strings ABI. >>> >>> | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit | >>> |-+-+-++| >>> | FULL | SSO-string | yes | + | + | >>> | | COW-string | yes | + | + | >>> |-+-+-++| >>> | PARTIAL | SSO-string | no | -+(*) | + | >>> | | COW-string | no | - | + | >>> *only longs strings are sanitized for 32-bit >>> >>> Some functions with new define looks a bit messy without changing >>> internal >>> functions(operator=), I'm also not sure if disabling string SSO >>> usage define >>> should also affects other parts that relies on basic_string class size >>> (checks >>> with static_assert in exceptions/shim-facets). >>> >>> >>> Any thoughts? >>> >>> On 05/29/2018 06:55 PM, Jonathan Wakely wrote: >>>> On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote: >>>>> Jonathan Wakely writes: >>>>>>> --- a/libstdc++-v3/include/bits/fstream.tcc >>>>>>> +++ b/libstdc++-v3/include/bits/fstream.tcc >>>>>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>>> >>>>>>> // Inhibit implicit instantiations for required instantiations, >>>>>>> // which are defined via explicit instantiations elsewhere. >>>>>>> +#if !_GLIBCXX_SANITIZE_STRING >>>>>>> #if _GLIBCXX_EXTERN_TEMPLATE >>>>>>> extern template class basic_filebuf; >>>>>>> extern template class basic_ifstream; >>>>>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>>> extern template class basic_fstream; >>>>>>> #endif >>>>>>> #endif >>>>>>> +#endif // !_GLIBCXX_SANITIZE_STRING >>>>>> Why do we need to disable these explicit instantiation declarations? >>>>>> Are they affected by the std::string layout changes? Is that just >>>>>> because of the constructors taking std::string, or something else? >>>>> Libstdc++ build is not sanitized, so macroses that requires >>>>> AddressSanitizer support will not applied and these templates will be >>>>> instantate without support for ASan annotations. >>>> Right, I was wondering specifically about the >>>> instantiations. I could be wrong but I don't think anything in >>>> creates, destroys or modifies a std::basic_string. >>>> >>>> >>>> >>>> >>>> > -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia
Re: [2/2] Add AddressSanitizer annotations to std::string.
^^^ gentle ping. On 08/16/2018 02:28 PM, Mikhail Kashkarov wrote: > ^^ gentle ping. > > On 07/16/2018 07:16 PM, Mikhail Kashkarov wrote: >> Rebased and update patch (typos, add missing annotations), >> add ASan teststo verify string annotation. >> >> >> On 06/28/2018 11:09 AM, Mikhail Kashkarov wrote: >>> ^ gentle ping. >>> >>> >>> On 06/08/2018 05:54 PM, Mikhail Kashkarov wrote: >>>> Hello, >>>> >>>> I've updated patches for std::string sanitization and disabling CXX11 >>>> string SSO usage for correct sanitization. >>>> >>>> >> _M_destroy(_M_allocated_capacity); >>>> >>+ else >>>> >>+ __annotate_delete(); >>>> > >>>> >Do these calls definitely optimize away completely when not >>>> >sanitizing? Even for -O1, -Os and -Og? >>>> > >>>> >For std::vector annotation I used macros to add these >>>> annotations, so >>>> >there is no change to the generated code when annotations are >>>> >disabled. But it makes the code quite ugly. >>>> >>>> I've checked asm code for string-inst.o and it looks like this >>>> calls are >>>> optimized away, but there are some light changes after patch fir . >>>> >>>> > Right, I was wondering specifically about the >>>> > instantiations. I could be wrong but I don't think anything in >>>> > creates, destroys or modifies a std::basic_string. >>>> >>>> I was confused by reinterpret_cast's on strings in fstream.tcc, looks >>>> like this is not needed, you are right. >>>> >>>> >> // calling 4.0.x+ _S_create. >>>> >> __p->_M_set_sharable(); >>>> >>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING >>>> >>+ __p->_M_length = 0; >>>> >>+#endif >>>> > >>>> > Why is this needed? I think a comment explaining it would help >>>> (like >>>> > the one above explaining why calling _M_set_sharable() is >>>> needed). >>>> >>>> Checked current version without this change, looks like now it works, >>>> reverted. >>>> >>>> Short summary: >>>> The reason for changing strings layout under sanitization is to >>>> place string >>>> char buffer on heap for correct aligning in 32-bit environments, >>>> both pre-CXX11 and CXX11 strings ABI. >>>> >>>> | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit | >>>> |-+-+-++| >>>> | FULL | SSO-string | yes | + | + | >>>> | | COW-string | yes | + | + | >>>> |-+-+-++| >>>> | PARTIAL | SSO-string | no | -+(*) | + | >>>> | | COW-string | no | - | + | >>>> *only longs strings are sanitized for 32-bit >>>> >>>> Some functions with new define looks a bit messy without changing >>>> internal >>>> functions(operator=), I'm also not sure if disabling string SSO >>>> usage define >>>> should also affects other parts that relies on basic_string class size >>>> (checks >>>> with static_assert in exceptions/shim-facets). >>>> >>>> >>>> Any thoughts? >>>> >>>> On 05/29/2018 06:55 PM, Jonathan Wakely wrote: >>>>> On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote: >>>>>> Jonathan Wakely writes: >>>>>>>> --- a/libstdc++-v3/include/bits/fstream.tcc >>>>>>>> +++ b/libstdc++-v3/include/bits/fstream.tcc >>>>>>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>>>> >>>>>>>> // Inhibit implicit instantiations for required >>>>>>>> instantiations, >>>>>>>> // which are defined via explicit instantiations elsewhere. >>>>>>>> +#if !_GLIBCXX_SANITIZE_STRING >>>>>>>> #if _GLIBCXX_EXTERN_TEMPLATE >>>>>>>> extern template class basic_filebuf; >>>>>>>> extern template class basic_ifstream; >>>>>>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>>>> extern template class basic_fstream; >>>>>>>> #endif >>>>>>>> #endif >>>>>>>> +#endif // !_GLIBCXX_SANITIZE_STRING >>>>>>> Why do we need to disable these explicit instantiation >>>>>>> declarations? >>>>>>> Are they affected by the std::string layout changes? Is that just >>>>>>> because of the constructors taking std::string, or something else? >>>>>> Libstdc++ build is not sanitized, so macroses that requires >>>>>> AddressSanitizer support will not applied and these templates >>>>>> will be >>>>>> instantate without support for ASan annotations. >>>>> Right, I was wondering specifically about the >>>>> instantiations. I could be wrong but I don't think anything in >>>>> creates, destroys or modifies a std::basic_string. >>>>> >>>>> >>>>> >>>>> >>>>> >> > -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia
Re: [1/2] Add option to disable c++11 std::string SSO
Updated. On 05/29/2018 09:53 AM, Mikhail Kashkarov wrote: > Add option to disable c++11 std::string small-string optimization usage. > > * include/bits/basic_string.h [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: > (basic_string::_M_is_local, basic_string::_M_destroy) > (basic_string::basic_string, basic_string::basic_string(const _Alloc&)) > (basic_string::basic_string(basic_string&&)) > (basic_string::basic_string(basic_string&&, const _Alloc&)) > (basic_string::operator=(const basic_string&)) > (basic_string::operator=(basic_string&&)) > (basic_string::clear()): Disable usage of _M_local_buf if > _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. > * include/bits/basic_string.tcc [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: > (basic_string::_M_construct, basic_string::reserve) > (basic_string::_M_replace): Disable usage of _M_local_buf if > _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. > * testsuite/basic_string/allocator/char/copy_assign.cc: Support for > std::string without SSO. > * testsuite/basic_string/allocator/wchar_t/copy_assign.cc: Likewise. > * testsuite/21_strings/basic_string/init-list.cc: Likewise. > * testsuite/rand/assoc/rand_regression_test.hpp: Likewise. > * testsuite/rand/priority_queue/rand_regression_test.hpp: Likewise. > -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia From 49c34919fba3000d751ac505b498eb6b21b0f4b3 Mon Sep 17 00:00:00 2001 From: Mikhail Kashkarov Date: Fri, 8 Jun 2018 12:22:33 +0300 Subject: [PATCH 1/2] Add option to disable c++11 std::basic_string SSO optimization. * include/bits/basic_string.h [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: (basic_string::_M_is_local()) (basic_string::_M_destroy(size_type __size)) (basic_string::basic_string()) (basic_string::basic_string(const _Alloc& __a)) (basic_string::basic_string(basic_string&& __str)) (basic_string::basic_string(basic_string&& __str, const _Alloc& __a)) (basic_string::operator=(const basic_string& __str)) (basic_string::operator=(basic_string&& __str)) (basic_string::clear()): Disable usage of _M_local_buf if _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. * include/bits/basic_string.tcc [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: (basic_string::_M_construct, basic_string::reserve) (basic_string::_M_replace): Disable usage of _M_local_buf if _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. * testsuite/basic_string/allocator/char/copy_assign.cc: Support for std::string without SSO. * testsuite/basic_string/allocator/wchar_t/copy_assign.cc: Likewise. * testsuite/21_strings/basic_string/init-list.cc: Likewise. * testsuite/rand/assoc/rand_regression_test.hpp: Likewise. * testsuite/rand/priority_queue/rand_regression_test.hpp: Likewise. --- libstdc++-v3/include/bits/basic_string.h | 104 +++-- libstdc++-v3/include/bits/basic_string.tcc | 46 - .../basic_string/allocator/char/copy_assign.cc | 4 + .../basic_string/allocator/wchar_t/copy_assign.cc | 4 + .../testsuite/21_strings/basic_string/init-list.cc | 2 + .../regression/rand/assoc/rand_regression_test.hpp | 5 + .../rand/priority_queue/rand_regression_test.hpp | 5 + 7 files changed, 156 insertions(+), 14 deletions(-) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 5bffa1c..9d971c0 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -208,7 +208,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 bool _M_is_local() const - { return _M_data() == _M_local_data(); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + return false; +#else + return _M_data() == _M_local_data(); +#endif + } // Create & Destroy pointer @@ -223,7 +229,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 void _M_destroy(size_type __size) throw() - { _Alloc_traits::deallocate(_M_get_allocator(), _M_data(), __size + 1); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + if (!_M_allocated_capacity) + return; +#endif +_Alloc_traits::deallocate(_M_get_allocator(), _M_data(), __size + 1); + } // _M_construct_aux is used to implement the 21.3.1 para 15 which // requires special behaviour if _InIterator is an integral type @@ -420,7 +432,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 basic_string() _GLIBCXX_NOEXCEPT_IF(is_nothrow_default_constructible<_Alloc>::value) : _M_dataplus(_M_local_data()) - { _M_set_length(0); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + _M_length(0); + _M_capacity(0); +#else + _M_set_length(0); +#endif + } /** * @brief Construct an empty string using allocator @a a. @@ -428,7 +447,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
Re: [2/2] Add AddressSanitizer annotations to std::string.
Hello, I've updated patches for std::string sanitization and disabling CXX11 string SSO usage for correct sanitization. >> _M_destroy(_M_allocated_capacity); >>+ else >>+ __annotate_delete(); > >Do these calls definitely optimize away completely when not >sanitizing? Even for -O1, -Os and -Og? > >For std::vector annotation I used macros to add these annotations, so >there is no change to the generated code when annotations are >disabled. But it makes the code quite ugly. I've checked asm code for string-inst.o and it looks like this calls are optimized away, but there are some light changes after patch fir . > Right, I was wondering specifically about the > instantiations. I could be wrong but I don't think anything in > creates, destroys or modifies a std::basic_string. I was confused by reinterpret_cast's on strings in fstream.tcc, looks like this is not needed, you are right. >> // calling 4.0.x+ _S_create. >> __p->_M_set_sharable(); >>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING >>+ __p->_M_length = 0; >>+#endif > > Why is this needed? I think a comment explaining it would help (like > the one above explaining why calling _M_set_sharable() is needed). Checked current version without this change, looks like now it works, reverted. Short summary: The reason for changing strings layout under sanitization is to place string char buffer on heap for correct aligning in 32-bit environments, both pre-CXX11 and CXX11 strings ABI. | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit | |-+-+-++| | FULL | SSO-string | yes | + | + | | | COW-string | yes | + | + | |-+-+-++| | PARTIAL | SSO-string | no | -+(*) | + | | | COW-string | no | - | + | *only longs strings are sanitized for 32-bit Some functions with new define looks a bit messy without changing internal functions(operator=), I'm also not sure if disabling string SSO usage define should also affects other parts that relies on basic_string class size (checks with static_assert in exceptions/shim-facets). Any thoughts? On 05/29/2018 06:55 PM, Jonathan Wakely wrote: > On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote: >> Jonathan Wakely writes: >>>> --- a/libstdc++-v3/include/bits/fstream.tcc >>>> +++ b/libstdc++-v3/include/bits/fstream.tcc >>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> >>>> // Inhibit implicit instantiations for required instantiations, >>>> // which are defined via explicit instantiations elsewhere. >>>> +#if !_GLIBCXX_SANITIZE_STRING >>>> #if _GLIBCXX_EXTERN_TEMPLATE >>>> extern template class basic_filebuf; >>>> extern template class basic_ifstream; >>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>> extern template class basic_fstream; >>>> #endif >>>> #endif >>>> +#endif // !_GLIBCXX_SANITIZE_STRING >>> >>> Why do we need to disable these explicit instantiation declarations? >>> Are they affected by the std::string layout changes? Is that just >>> because of the constructors taking std::string, or something else? >> >> Libstdc++ build is not sanitized, so macroses that requires >> AddressSanitizer support will not applied and these templates will be >> instantate without support for ASan annotations. > > Right, I was wondering specifically about the > instantiations. I could be wrong but I don't think anything in > creates, destroys or modifies a std::basic_string. > > > > > -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia From 4b8de0240ac091cdd43b690276f09e94bfb0ef4d Mon Sep 17 00:00:00 2001 From: Mikhail Kashkarov Date: Fri, 8 Jun 2018 14:11:11 +0300 Subject: [PATCH 2/2] Add AddressSanitizer annotations to std::string. * include/bits/c++config: define (_GLIBCXX_SANITIZE_STRING_PARTIAL, _GLIBCXX_SANITIZE_STRING_FULL) (_GLIBCXX_SANTIZE_STRING, _GLIBCXX_SANITIZER_ANNOTATE_STRING) (_GLIBCXX_SANITIZER_DISABLE_LOCAL_STRING_ANNOTATION) (_GLIBCXX_SANITIZER_ALIGN_COW_STRING) * doc/xml/manual/using.xml: document GLIBCXX_SANITIZE_STRING_PARTIAL, _GLIBCXX_SANITIZE_STRING_FULL * include/bits/basic_string.h [_GLIBCXX_USE_DUAL_ABI] (_asan_traits<_CharT, _Alloc>, _asan_traits<_CharT, allocator<_CharT>) (_asan_traits::__annotate_delete, _asan_traits::__annotate_new) (_asan_traits::__annotate_grow): New traits for annotation.
Re: [1/2] Add option to disable c++11 std::string small-size
Updated. On 05/29/2018 09:53 AM, Mikhail Kashkarov wrote: > Add option to disable c++11 std::string small-sizeoptimization usage. > > * include/bits/basic_string.h [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: > (basic_string::_M_is_local, basic_string::_M_destroy) > (basic_string::basic_string, basic_string::basic_string(const _Alloc&)) > (basic_string::basic_string(basic_string&&)) > (basic_string::basic_string(basic_string&&, const _Alloc&)) > (basic_string::operator=(const basic_string&)) > (basic_string::operator=(basic_string&&)) > (basic_string::clear()): Disable usage of _M_local_buf if > _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. > * include/bits/basic_string.tcc [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: > (basic_string::_M_construct, basic_string::reserve) > (basic_string::_M_replace): Disable usage of _M_local_buf if > _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. > * testsuite/basic_string/allocator/char/copy_assign.cc: Support for > std::string without SSO. > * testsuite/basic_string/allocator/wchar_t/copy_assign.cc: Likewise. > * testsuite/21_strings/basic_string/init-list.cc: Likewise. > * testsuite/rand/assoc/rand_regression_test.hpp: Likewise. > * testsuite/rand/priority_queue/rand_regression_test.hpp: Likewise. > -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia From 49c34919fba3000d751ac505b498eb6b21b0f4b3 Mon Sep 17 00:00:00 2001 From: Mikhail Kashkarov Date: Fri, 8 Jun 2018 12:22:33 +0300 Subject: [PATCH 1/2] Add option to disable c++11 std::basic_string SSO optimization. * include/bits/basic_string.h [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: (basic_string::_M_is_local()) (basic_string::_M_destroy(size_type __size)) (basic_string::basic_string()) (basic_string::basic_string(const _Alloc& __a)) (basic_string::basic_string(basic_string&& __str)) (basic_string::basic_string(basic_string&& __str, const _Alloc& __a)) (basic_string::operator=(const basic_string& __str)) (basic_string::operator=(basic_string&& __str)) (basic_string::clear()): Disable usage of _M_local_buf if _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. * include/bits/basic_string.tcc [_GLIBCXX_DISABLE_STRING_SSO_USAGE]: (basic_string::_M_construct, basic_string::reserve) (basic_string::_M_replace): Disable usage of _M_local_buf if _GLIBCXX_DISABLE_STRING_SSO_USAGE is defined. * testsuite/basic_string/allocator/char/copy_assign.cc: Support for std::string without SSO. * testsuite/basic_string/allocator/wchar_t/copy_assign.cc: Likewise. * testsuite/21_strings/basic_string/init-list.cc: Likewise. * testsuite/rand/assoc/rand_regression_test.hpp: Likewise. * testsuite/rand/priority_queue/rand_regression_test.hpp: Likewise. --- libstdc++-v3/include/bits/basic_string.h | 104 +++-- libstdc++-v3/include/bits/basic_string.tcc | 46 - .../basic_string/allocator/char/copy_assign.cc | 4 + .../basic_string/allocator/wchar_t/copy_assign.cc | 4 + .../testsuite/21_strings/basic_string/init-list.cc | 2 + .../regression/rand/assoc/rand_regression_test.hpp | 5 + .../rand/priority_queue/rand_regression_test.hpp | 5 + 7 files changed, 156 insertions(+), 14 deletions(-) diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 5bffa1c..9d971c0 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -208,7 +208,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 bool _M_is_local() const - { return _M_data() == _M_local_data(); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + return false; +#else + return _M_data() == _M_local_data(); +#endif + } // Create & Destroy pointer @@ -223,7 +229,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 void _M_destroy(size_type __size) throw() - { _Alloc_traits::deallocate(_M_get_allocator(), _M_data(), __size + 1); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + if (!_M_allocated_capacity) + return; +#endif +_Alloc_traits::deallocate(_M_get_allocator(), _M_data(), __size + 1); + } // _M_construct_aux is used to implement the 21.3.1 para 15 which // requires special behaviour if _InIterator is an integral type @@ -420,7 +432,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 basic_string() _GLIBCXX_NOEXCEPT_IF(is_nothrow_default_constructible<_Alloc>::value) : _M_dataplus(_M_local_data()) - { _M_set_length(0); } + { +#if _GLIBCXX_DISABLE_STRING_SSO_USAGE + _M_length(0); + _M_capacity(0); +#else + _M_set_length(0); +#endif + } /** * @brief Construct an empty string using allocator @a a. @@ -428,7 +447,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
Re: [2/2] Add AddressSanitizer annotations to std::string.
^ gentle ping. On 06/08/2018 05:54 PM, Mikhail Kashkarov wrote: > Hello, > > I've updated patches for std::string sanitization and disabling CXX11 > string SSO usage for correct sanitization. > > >> _M_destroy(_M_allocated_capacity); > >>+ else > >>+ __annotate_delete(); > > > >Do these calls definitely optimize away completely when not > >sanitizing? Even for -O1, -Os and -Og? > > > >For std::vector annotation I used macros to add these annotations, so > >there is no change to the generated code when annotations are > >disabled. But it makes the code quite ugly. > > I've checked asm code for string-inst.o and it looks like this calls are > optimized away, but there are some light changes after patch fir . > > > Right, I was wondering specifically about the > > instantiations. I could be wrong but I don't think anything in > > creates, destroys or modifies a std::basic_string. > > I was confused by reinterpret_cast's on strings in fstream.tcc, looks > like this is not needed, you are right. > > >> // calling 4.0.x+ _S_create. > >> __p->_M_set_sharable(); > >>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING > >>+ __p->_M_length = 0; > >>+#endif > > > > Why is this needed? I think a comment explaining it would help (like > > the one above explaining why calling _M_set_sharable() is needed). > > Checked current version without this change, looks like now it works, > reverted. > > Short summary: > The reason for changing strings layout under sanitization is to place string > char buffer on heap for correct aligning in 32-bit environments, > both pre-CXX11 and CXX11 strings ABI. > > | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit | > |-+-+-++| > | FULL | SSO-string | yes | + | + | > | | COW-string | yes | + | + | > |-+-+-++| > | PARTIAL | SSO-string | no | -+(*) | + | > | | COW-string | no | - | + | > *only longs strings are sanitized for 32-bit > > Some functions with new define looks a bit messy without changing internal > functions(operator=), I'm also not sure if disabling string SSO usage define > should also affects other parts that relies on basic_string class size > (checks > with static_assert in exceptions/shim-facets). > > > Any thoughts? > > On 05/29/2018 06:55 PM, Jonathan Wakely wrote: >> On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote: >>> Jonathan Wakely writes: >>>>> --- a/libstdc++-v3/include/bits/fstream.tcc >>>>> +++ b/libstdc++-v3/include/bits/fstream.tcc >>>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>> >>>>> // Inhibit implicit instantiations for required instantiations, >>>>> // which are defined via explicit instantiations elsewhere. >>>>> +#if !_GLIBCXX_SANITIZE_STRING >>>>> #if _GLIBCXX_EXTERN_TEMPLATE >>>>> extern template class basic_filebuf; >>>>> extern template class basic_ifstream; >>>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>> extern template class basic_fstream; >>>>> #endif >>>>> #endif >>>>> +#endif // !_GLIBCXX_SANITIZE_STRING >>>> Why do we need to disable these explicit instantiation declarations? >>>> Are they affected by the std::string layout changes? Is that just >>>> because of the constructors taking std::string, or something else? >>> Libstdc++ build is not sanitized, so macroses that requires >>> AddressSanitizer support will not applied and these templates will be >>> instantate without support for ASan annotations. >> Right, I was wondering specifically about the >> instantiations. I could be wrong but I don't think anything in >> creates, destroys or modifies a std::basic_string. >> >> >> >> >> -- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia
FW: [PATCH] PR fortran/104812: generate error for constuct-name clash with symbols
0001-Fortran-add-error-for-constuct-name-conflicts-with-s.patch Description: Binary data rcptInfo.txt Description: Binary data