On Thu, Jan 9, 2014 at 8:03 PM, Alp Toker <[email protected]> wrote: > > On 09/01/2014 15:23, Kostya Serebryany wrote: > >> >> On Thu, Jan 9, 2014 at 6:57 PM, Alp Toker <[email protected] <mailto: >> [email protected]>> 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? So, you propose to name this function lsan_is_turned_off (or some such) instead of __lsan_is_turned_off? That will work, but then there is a non-zero chance that a user will already have such function for whatever purposes of his own. What's worth, he will not know about the clash at the link time -- his program will start behaving wrongly. The __ prefix is exactly the tool that allows us to avoid a clash with user-named objects. Or I've got you wrong? > > > 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
