This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [2/2] Add AddressSanitizer annotations to std::string.
- From: Mikhail Kashkarov <m dot kashkarov at partner dot samsung dot com>
- To: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Jonathan Wakely <jwakely at redhat dot com>
- Date: Thu, 16 Aug 2018 14:28:49 +0300
- Subject: Re: [2/2] Add AddressSanitizer annotations to std::string.
- Cms-type: 201P
- Dkim-filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20180816112925euoutp02d2639ba9de1e30767dd63fbab8cb9d71~LWetE5IlT1753617536euoutp02g
- References: <c8b18d2a-d5c7-e3a2-53be-30b39eadeb49@partner.samsung.com> <0d8f122d-1376-5f25-4496-0d957eebdd7b@partner.samsung.com> <CGME20180529065519eucas1p1d2e1f33524940cdc783e059622769951@eucas1p1.samsung.com> <20180529065519eucas1p1d2e1f33524940cdc783e059622769951~zCx1IxDMB0595505955eucas1p1h@eucas1p1.samsung.com> <20180529141742.GJ7974@redhat.com> <20180529151835eucas1p20ee8861f37ca38d89830d2f4aa0d8fa8~zJpPxM4Zy0176201762eucas1p2a@eucas1p2.samsung.com> <20180529155513.GM7974@redhat.com> <20180608145440eucas1p22a8a8ad9d2c4283e191bd35e37ca9048~2NxOSIERn1904519045eucas1p2Y@eucas1p2.samsung.com> <20180628080959eucas1p117bf28bcdcbc94390ba58145f588a226~8RJll5sh_2049920499eucas1p1f@eucas1p1.samsung.com> <0ba29f7c-4156-d340-d719-a7d3aedf0d9f@partner.samsung.com>
^^ 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