r198858 - Disable LeakSanitizer in TableGen binaries, see PR18325
Alp Toker
alp at nuanti.com
Thu Jan 9 13:24:03 PST 2014
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
More information about the cfe-commits
mailing list