[llvm-commits] [ASan] add full strtoll implementation and interceptor (issue 5876052)

[Alexey Samsonov]

On Fri, Mar 23, 2012 at 12:25 AM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Thu, Mar 22, 2012 at 1:08 PM, <samsonov at google.com> wrote:
>
>> On 2012/03/22 16:15:11, kcc1 wrote:
>>
>>> I support the intention but I don't like the approach.
>>> You effectively reimplemented strtoll, which may appear to be even
>>>
>> more
>>
>>> complex function than it looks.
>>> Now we suddenly need to have an exhaustive test for this part of libc,
>>> including messy errno business.
>>> Meanwhile, why can't you use the original strtoll from libc?
>>> it gives you the endptr which could be used by asan run-time to detect
>>>
>> the
>>
>>> right bound of access.
>>>
>>
>> I'd also prefer to use libc strtoll, but we can't make our checks
>> complete in this case:
>> according to specification, strtoll sets endptr to the first char that
>> is not a part of the number OR the first char of the string if no digits
>> are found. So, if a string consists of just 100 whitespaces, endptr will
>> point to its beginning, while the implementation actually had to look
>> 100 symbols ahead. We can:
>> (1) don't handle this case (i.e. miss possible error reports) - in
>> general, this is not very good, as I think that memory errors are more
>> likely to occur in cases when there is no number somewhy.
>> (2) add some hacks to handle this case separately
>> (3) re-implement the whole thing.
>>
>> I'm not quite happy with the result of (3) either, but think we can get
>> away with this.
>
>
>
>
>> Or are you afraid of maintenance cost of this code?
>
>
> Exactly.
> I'd prefer to figure out the "last accessed byte" separately, based on the
> input and/or endptr.
>

OK, I see what you mean and will implement this.

It is fine if we report an un-addressable access after the strtoll call,
> not before.
>


>
>


>
> --kcc
>
>
>>
>>
>>
>>  Also, instead of ifndef(_WIN32) I would prefer more meaningful guards
>>>
>> like
>>
>>> #ifdef(ASAN_CAN_USE_STRTOLL), defined in one place.
>>>
>>
>>  --kcc
>>>
>>
>>  On Thu, Mar 22, 2012 at 5:17 AM, <mailto:samsonov at google.com> wrote:
>>>
>>
>>  > Reviewers: Evgeniy Stepanov, kcc1, timurrrr_at_google_com,
>>> >
>>> > Message:
>>> > We need to intercept strtol/atoi functions - calling them may result
>>>
>> in
>>
>>> > OOB reads (and they appear in stack traces of Chromium crash
>>>
>> reports).
>>
>>> > Looks like we have to re-implement them. Here's the major step to do
>>> > this.
>>> >
>>> >
>>> >
>>> > Please review this at
>>>
>>
>> http://codereview.appspot.com/****5876052/%3Chttp://**
>> codereview.appspot.com/**5876052/<http://codereview.appspot.com/**5876052/%3Chttp://codereview.appspot.com/5876052/>
>> >
>>
>>  >
>>> > Affected files:
>>> >  M     asan_interceptors.cc
>>> >  M     asan_interceptors.h
>>> >  M     asan_internal.h
>>> >  M     asan_posix.cc
>>> >  M     asan_rtl.cc
>>> >  M     asan_win.cc
>>> >  M     tests/asan_test.cc
>>> >
>>> >
>>> >
>>>
>>
>>
>>
>> http://codereview.appspot.com/**5876052/<http://codereview.appspot.com/5876052/>
>>
>
>


-- 
Alexey Samsonov
Software Engineer, Moscow
samsonov at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120323/c75c07ba/attachment.html>