^ 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
