[PATCH] D52251: [builtins] Add __emutls_unregister_key function

[Chih-Hung Hsieh via Phabricator via llvm-commits]

chh added inline comments.


================
Comment at: lib/builtins/emutls.c:397
+void __emutls_unregister_key(void) {
+    pthread_key_delete(emutls_pthread_key);
+}
----------------
rprichard wrote:
> kongyi wrote:
> > chh wrote:
> > > Shouldn't this be called only after the pthread key was created?
> > > gcc's version has a flag emutls_key_created set only after emutls_init.
> > > 
> > > 
> > We don't need to check this, since emutls_key_delete is not expensive to call and we can safely ignore failure from it.
> This code assumes that a zeroed pthread_key_t is never valid. I suspect that's true for Bionic? I confirmed it on aosp-master and KitKat. For the NDK's purposes, it needs to be true for android-16 and up.
> 
> It's apparently not generally true? http://www.tekkotsu.org/cvslog2web/2009/03/commit-1236808823.html
> 
My concern is run time error or crash in calls to pthread_key_delete(0).
That might not happen with bionic, but i think the behavior is undefined in POSIX and could happen in other platforms.
I know some other platforms already depend on this emutls.c.
Maybe they have not seen the dlclose issue yet, but it would be nice to make this function work for general cases.



Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52251