Re: libsanitizer merge from upstream r208536

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>]

On Fri, May 23, 2014 at 11:20 AM, Yury Gribov <y.gribov@samsung.com> wrote:
> On 05/23/2014 10:34 AM, Jakub Jelinek wrote:
>>>>
>>>> Otherwise libasan apps will simply stop
>>>> working altogether if LD_PRELOAD is set, to whatever library,
>>>> even if it doesn't define any symbols you care about.
>>>
>>>
>>> Right but I'm not sure whether failing fast here is necessarily bad.
>>
>>
>> I think it is very bad.  In fact, if you really want such a check,
>> I'd say it shouldn't be at least enabled by default, unless some
>> env var requests it; and document that if you are having troubles with
>> asan sanitized programs, try this magic env var to get better
>> troubleshooting.
>
>
> As I said, we can remove that Die() there. Warning can also be hidden under
> ASAN_OPTIONS=verbosity=1.
>
>
>> Even before this exaggerated check asan imposes far more restrictions than
>> good, and this just makes asan less usable just for fear that it wouldn't
>> work right.
>
>
> Ok, we seem to approach this from two different angles.
> I usually try to prohibit functionality that's not proven to work
> It could be later enabled if users need it.
>
>
>>
>> Most preloaded libs will just provide symbols asan never cares
>> about
>
>
> Maybe. In my experience it's all libc decorations.
>
>
>> even if you say override malloc completely without
>> calling the original implementation, the world doesn't end, the shadow
>> mem of those allocations just won't be surrounded by protected paddings,
>> so what, you don't detect out of bounds for malloc, but can still detect
>> out of bounds in your program's stack etc.
>> Ditto for string ops etc.
>
>
> Let's wait for Konstantin to comment on this.
> I don't know runtime well enough to guarantee that arbitrary symbol
> overloads are expected to work.

[These things are really better discussed at
address-sanitizer@googlegroups.com, where more asan people will read.
maybe we should start a separate topic there. This conversation is
already too long to comprehend. ]

Failing to intercept something may cause not just false negatives, but
also false positives.
These cases are often exceptionally hard to debug, so any checking that
the interception machinery works as intended is good. Of course if
these checks are wrong we should fix them.

>
>
>>> Imagine preloaded library has an initializer which calls intercepted
>>> APIs. Asan didn't get a chance to initialize at the point of call
>>> and if interceptor doesn't contain a sanity call to asan_init,
>>> we are risking hard-to-debug runtime error (call to NULL, etc.).
>>> I've seen numerous bugs like this (both locally and on mailing
>>> lists) and they were main motivation to add this check.
>>
>>
>> That is nonsense.  Early in the symbol search scope is the opposite of
>> being
>> initialized early, on the contrary, such libraries are initialized last.
>
>
> I may be wrong but my understanding was that ld.so performs a reverse
> topological sort of dependencies and initializes them in that order.
> Given that libasan depends on standard libs (librt, lipthread, libdl, etc.)
> it'll be initialized after them but before user libs.
> Initializers of std libs may indeed cause problems but we at least make sure
> to initialize before arbitrary user libraries.
>
>
>>> I wonder whether overriding Asan's malloc, etc. is expected to work
>>> at all? Perhaps banning it altogether is just the safest thing to
>>> do?
>>
>>
>> Don't know why you want to ban everything.
>
>
> To guarantee predictable and consistent behavior.
>
> -Y