Done at r198922.
On Fri, Jan 10, 2014 at 8:47 AM, Kostya Serebryany <[email protected]> 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 <[email protected]> wrote: > >> >> On 09/01/2014 21:12, Sean Silva wrote: >> >> >>> >>> >>> On Thu, Jan 9, 2014 at 12:49 PM, Alp Toker <[email protected] <mailto: >>> [email protected]>> 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 <[email protected] >>> <mailto:[email protected]> <mailto:[email protected] >>> >>> <mailto:[email protected]>>> wrote: >>> >>> >>> On 09/01/2014 17:30, Jordan Rose wrote: >>> >>> >>> On Jan 9, 2014, at 6:57 , Alp Toker <[email protected] >>> <mailto:[email protected]> <mailto:[email protected] >>> <mailto:[email protected]>><mailto:[email protected] >>> >>> <mailto:[email protected]>>> 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 >>> [email protected] <mailto:[email protected]> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >>> >> -- >> http://www.nuanti.com >> the browser experts >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
