r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325
kcc at google.com
Thu Jan 9 03:50:01 PST 2014
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
> 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
>> 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
> 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
We can easily add one, no problem, and then put this code under #ifdef
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
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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits