^^ 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 <fstream> >>> > instantiations. I could be wrong but I don't think anything in >>> > <fstream> 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 <jwak...@redhat.com> 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<char>; >>>>>>> extern template class basic_ifstream<char>; >>>>>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >>>>>>> extern template class basic_fstream<wchar_t>; >>>>>>> #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 <fstream> >>>> instantiations. I could be wrong but I don't think anything in >>>> <fstream> creates, destroys or modifies a std::basic_string. >>>> >>>> >>>> >>>> >>>> >
-- Best regards, Kashkarov Mikhail Samsung R&D Institute Russia
