On Jan 25, 2012, at 12:43 PM, Howard Hinnant wrote: > On Jan 25, 2012, at 12:28 PM, John McCall wrote: > >> On Jan 23, 2012, at 3:55 PM, Howard Hinnant wrote: >>> Author: hhinnant >>> Date: Mon Jan 23 17:55:58 2012 >>> New Revision: 148750 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=148750&view=rev >>> Log: >>> A lot of the code in cxa_exception.cpp depends on __cxa_get_globals_fast() >>> returning null if __cxa_get_globals() hasn't been called yet. However it >>> doesn't reliably do that, at least on OS X if __cxa_get_globals_fast() is >>> called prior to pthread_key_create() running. Our choice is to either >>> limit our use of __cxa_get_globals_fast() more than we have, or to have >>> __cxa_get_globals_fast() initialize with pthread_key_create() if necessary. >>> I chose the latter, and replaced pthread_once with a C++11 local static >>> (which should do the same thing). > > The use of so many pronouns is making me a little nervous, so I'm going to > try to confirm the intended use of each pronoun below. Please let me know if > I get it wrong. > >> Are you certain this function is not used in the implementation of >> _cxa_guard_acquire? > > I am certain that neither __cxa_get_globals_fast() nor __cxa_get_globals() is > used in the implementation of __cxa_guard_acquire(). I just checked and > cxa_guard.cpp does not currently even include the prototypes for > __cxa_get_globals_fast() and __cxa_get_globals(). > >> If so, please at least document in the guard-variables code that it is no >> longer allowed to call this. > > I'm not certain why the guard-variables code should be prohibited from > calling __cxa_get_globals_fast() or __cxa_get_globals(), though I don't > currently see the need either. > > My commit notes could probably have been clearer: This commit made calling > __cxa_get_globals_fast() safer, but slower, as opposed to the other way > around. You can now call __cxa_get_globals_fast() even if > __cxa_get_globals() has not previously been called. And if you do, you are > now guaranteed that __cxa_get_globals_fast() will return NULL. Previous to > this commit __cxa_get_globals_fast() would return a random non-NULL pointer > if called prior to __cxa_get_globals(). > > The previous state (returning a random non-NULL pointer) was conforming to > the Itanium ABI spec, but not consistent with our current use of > __cxa_get_globals_fast() (which expects a NULL pointer). > > If we're talking past each other, just let me know, and I'll try to clarify. > Thanks for the review.
As I hit send, I just realized your point: There is now a dependency of __cxa_get_globals_fast() on __cxa_guard_acquire() because of the use of the local static, and we don't want a circular dependency of __cxa_guard_acquire() on __cxa_get_globals_fast(). Good point. I'll comment cxa_guard.cpp. Howard _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
