r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325

Kostya Serebryany kcc at google.com
Mon Jan 13 07:02:29 PST 2014


Alp, others,

Now that there is no urgent issue I'd like to return to the discussion
about names with "__" prefix in sanitizer run-times.

We had __lsan_is_turned_off: a user of the tool has to define such symbol
to communicate some information to the tool.
Now there is an alternative name for this thing, but we still have a few
similar names with similar usage model elsewhere.

Examples:
compiler-rt/include/sanitizer/asan_interface.h
  const char* __asan_default_options();
  void __asan_malloc_hook(void *ptr, size_t size);
  void __asan_free_hook(void *ptr);

compiler-rt/include/sanitizer/msan_interface.h
  const char* __msan_default_options();
  void __msan_malloc_hook(const volatile void *ptr, size_t size);
  void __msan_free_hook(const volatile void *ptr);

compiler-rt/lib/lsan/lsan_common.h (should be moved to
compiler-rt/include/sanitizer)
  const char *__lsan_default_suppressions();

I may have missed a couple more.
These all have consistent naming with the rest of sanitizer interfaces.
This naming scheme is the only scheme known to me to that allows to avoid
clashes with user names.
And I still don't buy the arguments that defining functions with such names
in user code is illegal.
My understanding, is that the standard calls this "undefined behavior" and
we explicitly define this behavior for these particular function names.

It is very unlikely we will want to rename these existing functions to
something else,
and it is actually quite likely that we will want __lsan_is_turned_off (or
some such) back for consistency.
So we need to find a way to silence your tool for these names. Please
advise.
BTW, does the tool have a name and when will it become public?

--kcc




On Fri, Jan 10, 2014 at 12:11 PM, Kostya Serebryany <kcc at google.com> wrote:

> Done at r198922.
>
>
> On Fri, Jan 10, 2014 at 8:47 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>> Let me rename __lsan_is_turned_off to
>> LeakSanitizerIsTurnedOffForTheCurrentProcess and recommit the patch with
>> the new naming.
>> __lsan_is_turned_off will remain there as deprecated, so that we don't
>> break the existing uses. Will do this later today.
>> I don't buy Alp's reasons for revert, but the problem is just not worth
>> such a lengthy discussion.
>> Let's do something useful instead :)
>>
>> --kcc
>>
>>
>> On Fri, Jan 10, 2014 at 1:24 AM, Alp Toker <alp at nuanti.com> wrote:
>>
>>>
>>> On 09/01/2014 21:12, Sean Silva wrote:
>>>
>>>
>>>>
>>>>
>>>> On Thu, Jan 9, 2014 at 12:49 PM, Alp Toker <alp at nuanti.com <mailto:
>>>> alp at nuanti.com>> wrote:
>>>>
>>>>     OK, sorry I only just got round to this. Backed out in r198884 and
>>>>     r198885.
>>>>
>>>>     In principle it's OK to re-land this with the ifdef Jordan
>>>>     suggested, but I think it'd be more sustainable to try using
>>>>     non-reserved identifiers for the library part of the sanitizer
>>>>     interface if you have time to look into that.
>>>>
>>>>
>>>> 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.
>>>>
>>>> http://www.gnu.org/software/libc/manual/html_node/Hooks-for-Malloc.html
>>>> http://msdn.microsoft.com/en-us/library/c63a9b7h.aspx
>>>> __libc_malloc_dispatch in bionic libc
>>>>
>>>> The alternative is usually a function called by the user code in main()
>>>> or whatever, which takes a function pointer.
>>>>
>>>
>>> Yes, we discussed this earlier.
>>>
>>> That use case falls under permissible undefined behaviour, if and only
>>> if the implementation (of which the C library is a part) permits it.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Alp.
>>>
>>>
>>>
>>>>
>>>> -- Sean Silva
>>>>
>>>>
>>>>     Cheers
>>>>     Alp.
>>>>
>>>>
>>>>
>>>>     On 09/01/2014 17:52, Jordan Rose wrote:
>>>>
>>>>
>>>>         On Jan 9, 2014, at 9:42 , Alp Toker <alp at nuanti.com
>>>>         <mailto:alp at nuanti.com> <mailto:alp at nuanti.com
>>>>
>>>>         <mailto:alp at nuanti.com>>> wrote:
>>>>
>>>>
>>>>             On 09/01/2014 17:30, Jordan Rose wrote:
>>>>
>>>>
>>>>                 On Jan 9, 2014, at 6:57 , Alp Toker <alp at nuanti.com
>>>>                 <mailto:alp at nuanti.com> <mailto:alp at nuanti.com
>>>>                 <mailto:alp at nuanti.com>><mailto:alp at nuanti.com
>>>>
>>>>                 <mailto:alp at nuanti.com>>> wrote:
>>>>
>>>>                     I'm not making an assertion. The standard is very
>>>>                     clear on this:
>>>>
>>>>
>>>>                        *17.6.4.3 Reserved names [reserved.names]*
>>>>
>>>>                        If a program declares or defines a name in a
>>>>                     context where it is
>>>>                        reserved, other than as explicitly allowed by
>>>>                     this Clause, its
>>>>                        *behavior is undefined*.
>>>>
>>>>                        *17.6.4.3.2 Global names [global.names]*
>>>>
>>>>                        Certain sets of names and function signatures
>>>>                     are always reserved
>>>>                        to the implementation:
>>>>
>>>>                          * *Each name that contains a double
>>>>                     underscore __*or begins
>>>>                            with an underscore followed by an uppercase
>>>>                     letter (2.12) *is
>>>>                            reserved to the implementation for any use*.
>>>>                          * Each name that begins with an underscore is
>>>>                     reserved to the
>>>>                            implementation for use as a name in the
>>>>                     global namespace.
>>>>
>>>>
>>>>
>>>>                 I know I shouldn't be getting into this, but...
>>>>
>>>>                    *1.3.24 undefined behavior [defns.undefined]*
>>>>                    behavior for which this International Standard
>>>>                 imposes no requirements
>>>>                    /[ Note: Undefined behavior may be expected when this
>>>>                    International Standard omits any explicit
>>>>                 definition of behavior
>>>>                    or when a program uses an erroneous construct or
>>>>                 erroneous data.
>>>>                    *Permissible undefined behavior* ranges from
>>>>                 ignoring the
>>>>                    situation completely with unpredictable results,
>>>>                 *to behaving
>>>>                    during translation or program execution in a
>>>>                 documented manner
>>>>                    characteristic of the environment* (with or without
>>>>                 the issuance
>>>>                    of a diagnostic message), to terminating a
>>>>                 translation or
>>>>                    execution (with the issuance of a diagnostic
>>>>                 message). Many
>>>>                    erroneous program constructs do not engender
>>>> undefined
>>>>                    behavior; they are required to be diagnosed. — end
>>>>                 note ]/
>>>>
>>>>
>>>>                 (emphasis mine)
>>>>
>>>>                 As I read this, a valid interpretation of this program
>>>>                 is that when LEAK_SANITIZER is defined, the program
>>>>                 contains undefined behavior, and therefore it should
>>>>                 only be set in a context when the particular
>>>>                 implementation is known to do something sensible for
>>>>                 this particular undefined behavior (that is, use the
>>>>                 function at runtime to disable leak checking).
>>>>
>>>>                 I don't see this as abstractly different from the
>>>>                 standard-specified practice of replacing the global
>>>>                 operator new, so I don't think it's inherently an
>>>>                 anti-pattern. I think everyone agrees on this since
>>>>                 you've said already you'd have no objections if the
>>>>                 name weren't one of the restricted [global.names] names.
>>>>
>>>>                 Would it help if the function were pre-declared in a
>>>>                 system header, and then just implemented or not
>>>>                 implemented in user code?
>>>>
>>>>
>>>>             Hi Jordan,
>>>>
>>>>             That's the current situation -- as long as sanitizer
>>>>             headers are available and in use the part of the spec you
>>>>             highlighted means it's acceptable.
>>>>
>>>>             The problem arises when sanitizer headers aren't available.
>>>>
>>>>
>>>>         I just don't think the program is illegal when LEAK_SANITIZER
>>>>         is not defined. The tokens within the #ifdef are skipped
>>>>         completely, so they don't refer to names and certainly don't
>>>>         declare anything.
>>>>
>>>>         I'm not sure we should care about the case where
>>>>         LEAK_SANITIZER is defined in an environment that doesn't
>>>>         specify what defining this particular name will do. The user
>>>>         has full control over this. (And in fact, IIRC being able to
>>>>         define macros on the command line isn't at all specified by
>>>>         the standard, so the program by itself will /always/ skip the
>>>>         LEAK_SANITIZER block.)
>>>>
>>>>         Jordan
>>>>
>>>>
>>>>     --     http://www.nuanti.com
>>>>     the browser experts
>>>>
>>>>     _______________________________________________
>>>>     cfe-commits mailing list
>>>>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>>
>>> --
>>> http://www.nuanti.com
>>> the browser experts
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140113/222280e7/attachment.html>


More information about the cfe-commits mailing list