This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Allocate extra 16 bytes for -fsanitize=address


On Fri, Nov 23, 2012 at 10:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Nov 23, 2012 at 9:38 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Fri, Nov 23, 2012 at 09:23:37AM -0800, H.J. Lu wrote:
>>> This patch allocates extra 16 bytes for -fsanitize=address so that
>>> asan won't report read beyond memory buffer. It is used by
>>> bootstrap-asan.  OK to install?
>>
>> As valgrind warns about that too, I'd say we should do that unconditionally,
>> the additional 16-bytes just aren't a big deal.
>
> This isn't sufficient for valgrind since valgrind will also report
> reading uninitialized data, which requires additional memset
> on extra 16 bytes.

Valgrind should not report reading uninitialized data if that data is
actually not used later (extra bytes are cleared by AND-ing with
zeroes).
If the code was mine, I would simply use (to.len + 17) w/o any conditionals.

--kcc


>
>> But, _cpp_convert_input isn't the only place which needs that, IMHO you want
>
> This patch is sufficient to compile libcpp with -fsanitize=address.
>
>> to also change the caller, read_file_guts, where it does
>>   buf = XNEWVEC (uchar, size + 1);
>> and
>>           buf = XRESIZEVEC (uchar, buf, size + 1);
>
> I don't think it is necessary since there is no real data in
> those extra 16 bytes.  They can have garbage and libcpp still
> functions correctly.  They are purely used to silence ASAN.
>
>> I'll defer the review to Tom though.
>>
>>> 2012-11-21  H.J. Lu  <hongjiu.lu@intel.com>
>>>
>>>       PR bootstrap/55380
>>>       * charset.c (_cpp_convert_input): Allocate extra 16 bytes for
>>>       -fsanitize=address if __SANITIZE_ADDRESS__ is defined.
>>>
>>> diff --git a/libcpp/charset.c b/libcpp/charset.c
>>> index cba19a6..dea8bb1 100644
>>> --- a/libcpp/charset.c
>>> +++ b/libcpp/charset.c
>>> @@ -1729,9 +1729,16 @@ _cpp_convert_input (cpp_reader *pfile, const char *input_charset,
>>>      iconv_close (input_cset.cd);
>>>
>>>    /* Resize buffer if we allocated substantially too much, or if we
>>> -     haven't enough space for the \n-terminator.  */
>>> +     haven't enough space for the \n-terminator.  Allocate extra 16
>>> +     bytes for -fsanitize=address.  */
>>>    if (to.len + 4096 < to.asize || to.len >= to.asize)
>>> -    to.text = XRESIZEVEC (uchar, to.text, to.len + 1);
>>> +    {
>>> +#ifdef __SANITIZE_ADDRESS__
>>> +      to.text = XRESIZEVEC (uchar, to.text, to.len + 17);
>>> +#else
>>> +      to.text = XRESIZEVEC (uchar, to.text, to.len + 1);
>>> +#endif
>>> +    }
>>>
>>>    /* If the file is using old-school Mac line endings (\r only),
>>>       terminate with another \r, not an \n, so that we do not mistake
>>
>>         Jakub
>
>
>
> --
> H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]