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

[Kostya Serebryany]

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.
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/>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120322/b5ebfd3b/attachment.html>