r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325

[Alp Toker]

On 09/01/2014 15:23, Kostya Serebryany wrote:
>
> On Thu, Jan 9, 2014 at 6:57 PM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>
>     On 09/01/2014 12:13, Chandler Carruth wrote:
>>
>>         Who controls / defines the interface?
>>
>>
>>     The sanitizers.
>
>     Can you report a bug with them?
>
>
> That's us (no need to file a separate bug unless you want to)

That's fantastic. A proper resolution might be in sight in that case..

>
>
>     Moreover, it's blocking work on a practical level by triggering
>     legitimate build issues in at least one configuration. Removing
>     the function doesn't break anything and resolves the issues. I
>     don't see the problem here?
>
>
> Will you get unblocked if we put this code under #ifdef LEAK_SANITIZER ?
> Alternatively you may probably build the code with 
> -D__lsan_is_turned_off=something_else

If conditionalizing turns out to be necessary, it seems best to not to 
enable the function definition unless it's explicitly leak sanitizer build.

However I'm starting to understand what you were trying to achieve and 
have a much better proposal below..

>
> Do you have suggestions how to improve the interface

We should be able to find a name or mechanisms to do this properly, 
given that you have control over the interface.

> w/o risking to clash with user-defined names?

That right there appears to be the point of confusion.

The problem is that you've required users to declare a function name 
that should have been reserved for the implementation.

I said earlier this appears to be a "thinko" in the sanitizer interface. 
I said that because, for the majority of sanitizer functions, you're 
dealing with the reverse problem, and that appears to have leaked into 
the naming of these functions unintentionally.

So it's not a question of avoiding a clash with user-defined names, but 
the other way round.

In summary, the problem you need to solve is identical to the naming 
choices when declaring an entry point in a public C programming interface.

Try naming this like any C API function (which is what it is) and you'll 
have a solution. Some good LLVM-style function name conventions are 
"include/llvm-c/Core.h" assuming you want to go with the LLVM CamelCase 
style.

With this approach, you have the same risk of conflict as any C 
function, which is a well-understood problem space. The sanitizer is 
then free to give this symbol name special handling internally without 
imposing on others.

If you do this there's no need to conditionalize, but please retain the 
comment explaining the purpose so it doesn't get "cleaned up".

Thanks for inviting a discussion Kostya. Does that sound good to you?

Alp.



>
> --kcc
>
>
>     Anyway, I'm returning to the office to deal with this and I'm
>     disappointed you've tried to push it through unilaterally with
>     disregard for anyone outside of your own circle.
>
>
>     Alp.
>
>     -- 
>     http://www.nuanti.com
>     the browser experts
>
>

-- 
http://www.nuanti.com
the browser experts