<div dir="ltr"><div>Alp, others, </div><div><br></div>Now that there is no urgent issue I'd like to return to the discussion about names with "__" prefix in sanitizer run-times.<div><br><div>We had __lsan_is_turned_off: a user of the tool has to define such symbol to communicate some information to the tool.</div>
</div><div>Now there is an alternative name for this thing, but we still have a few similar names with similar usage model elsewhere. </div><div><br></div><div>Examples: </div><div>compiler-rt/include/sanitizer/asan_interface.h<br>
</div><div><div> const char* __asan_default_options();</div></div><div><div> void __asan_malloc_hook(void *ptr, size_t size);</div><div> void __asan_free_hook(void *ptr);</div></div><div><br></div><div>compiler-rt/include/sanitizer/msan_interface.h<br>
</div><div><div> const char* __msan_default_options();</div></div><div> void __msan_malloc_hook(const volatile void *ptr, size_t size);<br></div><div><div> void __msan_free_hook(const volatile void *ptr);</div></div><div>
<br></div><div>compiler-rt/lib/lsan/lsan_common.h (should be moved to compiler-rt/include/sanitizer)<br></div><div><div> const char *__lsan_default_suppressions();</div></div><div><br></div><div>I may have missed a couple more. </div>
<div>These all have consistent naming with the rest of sanitizer interfaces.</div><div>This naming scheme is the only scheme known to me to that allows to avoid clashes with user names. </div><div>And I still don't buy the arguments that defining functions with such names in user code is illegal.</div>
<div>My understanding, is that the standard calls this "undefined behavior" and we explicitly define this behavior for these particular function names.</div><div><br></div><div>It is very unlikely we will want to rename these existing functions to something else,</div>
<div>and it is actually quite likely that we will want __lsan_is_turned_off (or some such) back for consistency.</div><div>So we need to find a way to silence your tool for these names. Please advise.</div><div>BTW, does the tool have a name and when will it become public?<br>
</div><div><br></div><div>--kcc </div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jan 10, 2014 at 12:11 PM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Done at r198922.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">
On Fri, Jan 10, 2014 at 8:47 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Let me rename __lsan_is_turned_off to LeakSanitizerIsTurnedOffForTheCurrentProcess and recommit the patch with the new naming. <div>
<div>__lsan_is_turned_off will remain there as deprecated, so that we don't break the existing uses. Will do this later today. </div>
<div>I don't buy Alp's reasons for revert, but the problem is just not worth such a lengthy discussion. </div><div>Let's do something useful instead :) </div><div><br></div><div>--kcc </div></div></div><div>
<div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Jan 10, 2014 at 1:24 AM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On 09/01/2014 21:12, Sean Silva wrote:<div><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<br>
On Thu, Jan 9, 2014 at 12:49 PM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>> wrote:<br>
<br>
OK, sorry I only just got round to this. Backed out in r198884 and<br>
r198885.<br>
<br>
In principle it's OK to re-land this with the ifdef Jordan<br>
suggested, but I think it'd be more sustainable to try using<br>
non-reserved identifiers for the library part of the sanitizer<br>
interface if you have time to look into that.<br>
<br>
<br>
I'm not sure if you're aware of this Alp, but using a reserved "weak" symbol is a classic and widely used way to expose "hooks" into runtime libraries/instrumentation.<br>
<br>
<a href="http://www.gnu.org/software/libc/manual/html_node/Hooks-for-Malloc.html" target="_blank">http://www.gnu.org/software/<u></u>libc/manual/html_node/Hooks-<u></u>for-Malloc.html</a><br>
<a href="http://msdn.microsoft.com/en-us/library/c63a9b7h.aspx" target="_blank">http://msdn.microsoft.com/en-<u></u>us/library/c63a9b7h.aspx</a><br>
__libc_malloc_dispatch in bionic libc<br>
<br>
The alternative is usually a function called by the user code in main() or whatever, which takes a function pointer.<br>
</blockquote>
<br></div>
Yes, we discussed this earlier.<br>
<br>
That use case falls under permissible undefined behaviour, if and only if the implementation (of which the C library is a part) permits it.<br>
<br>
Not just bionic libc but glibc and others system components including the sanitizer control function in discussion have weak symbols, often prefixed "__" like this exposed as hooks which user code is free to define and override.<br>
<br>
What's not OK is to define the reserved names when a _different_ C library or implementation is in use, and the wording in the standard makes it clear the intention is to prevent implementations stepping on one another.<br>
<br>
Alp.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<br>
<br>
-- Sean Silva<br>
<br>
<br>
Cheers<br>
Alp.<br>
<br>
<br>
<br>
On 09/01/2014 17:52, Jordan Rose wrote:<br>
<br>
<br>
On Jan 9, 2014, at 9:42 , Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br></div>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><div><br>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>>> wrote:<br>
<br>
<br>
On 09/01/2014 17:30, Jordan Rose wrote:<br>
<br>
<br>
On Jan 9, 2014, at 6:57 , Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br></div>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>><<u></u>mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><div><div><br>
<mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>>> wrote:<br>
<br>
I'm not making an assertion. The standard is very<br>
clear on this:<br>
<br>
<br>
*17.6.4.3 Reserved names [reserved.names]*<br>
<br>
If a program declares or defines a name in a<br>
context where it is<br>
reserved, other than as explicitly allowed by<br>
this Clause, its<br>
*behavior is undefined*.<br>
<br>
*17.6.4.3.2 Global names [global.names]*<br>
<br>
Certain sets of names and function signatures<br>
are always reserved<br>
to the implementation:<br>
<br>
* *Each name that contains a double<br>
underscore __*or begins<br>
with an underscore followed by an uppercase<br>
letter (2.12) *is<br>
reserved to the implementation for any use*.<br>
* Each name that begins with an underscore is<br>
reserved to the<br>
implementation for use as a name in the<br>
global namespace.<br>
<br>
<br>
<br>
I know I shouldn't be getting into this, but...<br>
<br>
*1.3.24 undefined behavior [defns.undefined]*<br>
behavior for which this International Standard<br>
imposes no requirements<br>
/[ Note: Undefined behavior may be expected when this<br>
International Standard omits any explicit<br>
definition of behavior<br>
or when a program uses an erroneous construct or<br>
erroneous data.<br>
*Permissible undefined behavior* ranges from<br>
ignoring the<br>
situation completely with unpredictable results,<br>
*to behaving<br>
during translation or program execution in a<br>
documented manner<br>
characteristic of the environment* (with or without<br>
the issuance<br>
of a diagnostic message), to terminating a<br>
translation or<br>
execution (with the issuance of a diagnostic<br>
message). Many<br>
erroneous program constructs do not engender undefined<br>
behavior; they are required to be diagnosed. — end<br>
note ]/<br>
<br>
<br>
(emphasis mine)<br>
<br>
As I read this, a valid interpretation of this program<br>
is that when LEAK_SANITIZER is defined, the program<br>
contains undefined behavior, and therefore it should<br>
only be set in a context when the particular<br>
implementation is known to do something sensible for<br>
this particular undefined behavior (that is, use the<br>
function at runtime to disable leak checking).<br>
<br>
I don't see this as abstractly different from the<br>
standard-specified practice of replacing the global<br>
operator new, so I don't think it's inherently an<br>
anti-pattern. I think everyone agrees on this since<br>
you've said already you'd have no objections if the<br>
name weren't one of the restricted [global.names] names.<br>
<br>
Would it help if the function were pre-declared in a<br>
system header, and then just implemented or not<br>
implemented in user code?<br>
<br>
<br>
Hi Jordan,<br>
<br>
That's the current situation -- as long as sanitizer<br>
headers are available and in use the part of the spec you<br>
highlighted means it's acceptable.<br>
<br>
The problem arises when sanitizer headers aren't available.<br>
<br>
<br>
I just don't think the program is illegal when LEAK_SANITIZER<br>
is not defined. The tokens within the #ifdef are skipped<br>
completely, so they don't refer to names and certainly don't<br>
declare anything.<br>
<br>
I'm not sure we should care about the case where<br>
LEAK_SANITIZER is defined in an environment that doesn't<br>
specify what defining this particular name will do. The user<br>
has full control over this. (And in fact, IIRC being able to<br>
define macros on the command line isn't at all specified by<br>
the standard, so the program by itself will /always/ skip the<br>
LEAK_SANITIZER block.)<br>
<br>
Jordan<br>
<br>
<br>
-- <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br></div></div>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
<br>
<br>
</blockquote><div><div>
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>