[2/2] Add AddressSanitizer annotations to std::string.

[Mikhail Kashkarov]

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 <jwakely@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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-AddressSanitizer-annotations-to-std-string.patch
Type: text/x-patch
Size: 35466 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20180608/2081a1fb/attachment.bin>