r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325
Kostya Serebryany
kcc at google.com
Thu Jan 9 20:47:49 PST 2014
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/1c54e9d3/attachment.html>
More information about the cfe-commits
mailing list