r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325

[Kostya Serebryany]

On Thu, Jan 9, 2014 at 3:36 PM, Chandler Carruth <chandlerc at google.com>wrote:

>
> On Thu, Jan 9, 2014 at 3:15 AM, Alp Toker <alp at nuanti.com> wrote:
>
>> __builtin functions are introduced by the implementation (either the
>> compiler or system header declarations) so there's no problem.
>>
>> Sanitizer is a system library so it's free to use the identifiers it
>> wants.
>>
>
> The sanitizers aren't system libraries as I define them (things in
> /usr/include or /usr/lib). They're compiler runtime libraries. They get
> installed in lib/clang/... along with other compiler resources. They are
> tightly coupled and version locked with the compiler itself as they
> interface with compiler generated instrumentation in most cases. The leak
> sanitizer is unusual in that it *could* be separated into something other
> than the compiler runtime libraries, but we explicitly chose not to as a
> design goal: we want to be able to offer it coupled with address sanitizer.
>
> Also, system libraries aren't free to use reserved names... so maybe you
> meant something different from what is commonly called a system library on
> linux/mac?
>
>
>> TableGen, on the other hand, is not a part of the implementation.
>>
>
> No one is claiming that it is.
>
>
>>
>>
>>
>>> Can you clarify exactly what the error is and why? It's possible we
>>> could provide a macro of some kind in the sanitizer headers to bundle this
>>> functionality up if we need some special marking of this for checkers, but
>>> without more information its hard to hell what the actual problem is.
>>>
>>
>> These two commits _introduced_ names in the reserved namespace.
>>
>
> Reserved names are reserved for any use. That can include an
> implementation specifically blessing the introduction of a name that uses
> double underscores. It may be unusual, but I don't think there is anything
> in the standard that makes this a non-conforming extension.
>
>
>> The checker I have is a strict warning flag I've been trying to encode
>> the rules from the standard and in this case the diagnostic looks
>> legitimate.
>>
>
> Ok, well, we disagree. ;]
>
> If the concern is that this is an ugly interface, I'm somewhat sympathetic
> and we could explore better ways to expose this interface to users, but I
> don't think this is incorrect as is, it just looks slightly sub-optimal.
> And I don't think there is any standards problem here, as this is and
> intentional extension provided by Clang+LSan.
>
> Now, one thing that I meant to ask for and didn't is to hid all of this
> behind some macro which is used to enable LSan. If we're not compiling with
> a compiler that supports this extension, it seems reasonable (if not likely
> of practical importance) to avoid touching the reserved name.
>

We currently don't have any macro that is used to enable lsan in llvm
bootstrap.
We can easily add one, no problem, and then put this code under #ifdef
LEAK_SANITIZER
It would make things a bit more ugly.

We have tow problems to solve wrt lsan and bootstrap:
1. Make asan+lsan bootstrap clean. This is what I need to solve now to
enable leak checking for the entire llvm. For this we may also use #if
__has_feature(address_sanitizer).
2. Make any+lsan bootstrap clean ("any" is one of asan, tsan, msan,
<none>). This may introduce more complexity and that is why the current
solution is nice. I don't have to solve this general problem though for the
asan+lsan bootstrap bot.

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140109/966029c3/attachment.html>