r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325
Kostya Serebryany
kcc at google.com
Fri Jan 10 00:11:06 PST 2014
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/20140110/5ee96f7a/attachment.html>
More information about the cfe-commits
mailing list