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.

Howard

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to