[2/2] Add AddressSanitizer annotations to std::string.
Mikhail Kashkarov
m.kashkarov@partner.samsung.com
Tue Sep 4 13:37:00 GMT 2018
^^^ 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 <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
More information about the Gcc-patches
mailing list