[PATCH] D17891: [sanitizer] Fix strlen assumptions in sanitizer interception code

[Kostya Serebryany via llvm-commits]

On Fri, Mar 4, 2016 at 11:48 AM, Derek Bruening <bruening at google.com> wrote:

> On Fri, Mar 4, 2016 at 2:34 PM, Kostya Serebryany <kcc at google.com> wrote:
>
>>
>> On Fri, Mar 4, 2016 at 11:32 AM, Derek Bruening <bruening at google.com>
>> wrote:
>>
>>> bruening added a comment.
>>>
>>> What if instead we moved the strlen interceptors that are currently
>>> private to asan, tsan, and msan into the set of common interceptors, thus
>>> ensuring that REAL(strlen) exists (as well as unifying the currently
>>> duplicate interceptors)?  I actually already have a local CL that moves
>>> strlen and strchr* into the common set which I can send for review.
>>>
>> In most cases such unification is more than welcome!
>> (Just need to be careful not to lose any functionality)
>>
>
> Putting more into the common set will make it easier to add new tools as
> well.
>
> Perhaps you could give your thoughts on some of the complexities involved:
>
>    - Are you ok with asan's replace_str flag applying not just to "string
>    routines" but also to string-typed parameters (e.g., the first parameter to
>    glob())?
>
> Not really. Let's leave replace_str as as and add new flags for other
(classes of) interceptors if needed.
This flags are unfortunate, but required -- different users have different
sets of bugs they can not fix. :(



>
>    - Currently, asan's replace_str flag does not apply to string routines
>    already in the common set (AFAICT).  Presumably this is not intentional.
>
> semi-intentional. :)
replace_str was a mistake, we really needed a separate flag for every
interceptor (or at least a class if related interceptors, like *printf*)


>    - Are you ok with adding interceptors to a sanitizer that were not
>    there before: e.g., putting strchr into the common set will add it to msan
>    (it's already in asan and tsan). Is there additional testing that must be
>    done in such cases beyond the llvm tests?
>
> More than OK, it's welcome!
As long as every new interceptor is under it's own flag (or a class of
related new interceptors under a single flag)
it's ok even if the flag is on by default (preferred).
If such a new interceptor cases a new failure we will be able to disable it
quickly (while not disabling the old interceptors).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160304/543c204b/attachment.html>