OSDN Git Service

Revert "Revert "Remove __sinit and __sdidinit.""
authorElliott Hughes <enh@google.com>
Sat, 5 Dec 2015 02:03:12 +0000 (18:03 -0800)
committerElliott Hughes <enh@google.com>
Sat, 5 Dec 2015 15:30:59 +0000 (07:30 -0800)
This reverts commit c8bae05f3ff9f1c736f7be70fa17d02795d748bb.

We were breaking init (ueventd) because we initialize system properties
before we initialize stdio. The new system property implementation uses
stdio to read from /property_contexts, so we end up touching stdio data
structures before they've been initialized.

This second attempt takes things further by removing the stdio initialization
function altogether. The data structures for stdin/stdout/stderr can be
statically initialized as data, and -- since we already had to give the
atexit implementation a backdoor for stdio -- we can just admit that we
need to clean up stdio, and that we always do so last.

This patch also removes the 17 statically pre-allocated file structures,
so the first fopen will now allocate a block of 10 (the usual overflow
behavior). I did this just to make my life simpler, but it's not actually
necessary to remove it if we want it back.

Change-Id: I936b2eb5e88e4ebaf5516121872b71fc88e5609c

libc/bionic/flockfile.cpp
libc/bionic/libc_init_common.cpp
libc/bionic/ndk_cruft.cpp
libc/stdio/findfp.c
libc/stdio/local.h
libc/stdio/refill.c
libc/stdlib/atexit.c

index db68801..db53828 100644 (file)
 // struct __sfileext (see fileext.h).
 
 void flockfile(FILE* fp) {
-  if (!__sdidinit) {
-    __sinit();
-  }
-
   if (fp != nullptr) {
     pthread_mutex_lock(&_FLOCK(fp));
   }
 }
 
 int ftrylockfile(FILE* fp) {
-  if (!__sdidinit) {
-    __sinit();
-  }
-
   // The specification for ftrylockfile() says it returns 0 on success,
   // or non-zero on error. So return an errno code directly on error.
   if (fp == nullptr) {
@@ -60,10 +52,6 @@ int ftrylockfile(FILE* fp) {
 }
 
 void funlockfile(FILE* fp) {
-  if (!__sdidinit) {
-    __sinit();
-  }
-
   if (fp != nullptr) {
     pthread_mutex_unlock(&_FLOCK(fp));
   }
index 91e210e..8f1ee95 100644 (file)
@@ -54,7 +54,6 @@ extern "C" abort_msg_t** __abort_message_ptr;
 extern "C" int __system_properties_init(void);
 extern "C" int __set_tls(void* ptr);
 extern "C" int __set_tid_address(int* tid_address);
-extern "C" int __sinit(void);
 
 __LIBC_HIDDEN__ WriteProtected<libc_globals> __libc_globals;
 
@@ -134,9 +133,6 @@ void __libc_init_common(KernelArgumentBlock& args) {
   __pthread_internal_add(main_thread);
 
   __system_properties_init(); // Requires 'environ'.
-  // Initialize stdio here to get rid of data races caused by lazy initialization.
-  // TODO: Remove other calls to __sinit().
-  __sinit();
 }
 
 __noreturn static void __early_abort(int line) {
index d6b8e8f..0af9d5b 100644 (file)
@@ -286,6 +286,13 @@ extern "C" int getdtablesize() {
   return r.rlim_cur;
 }
 
+// A leaked BSD stdio implementation detail that's now a no-op.
+// (GCC doesn't like 'extern "C"' on a definition.)
+extern "C" {
+void __sinit() {}
+int __sdidinit = 1;
+}
+
 // Only used by ftime, which was removed from POSIX 2008.
 struct timeb {
   time_t          time;
index 2696cfd..35de072 100644 (file)
 #define ALIGNBYTES (sizeof(uintptr_t) - 1)
 #define ALIGN(p) (((uintptr_t)(p) + ALIGNBYTES) &~ ALIGNBYTES)
 
-int    __sdidinit;
-
 #define        NDYNAMIC 10             /* add ten more whenever necessary */
 
 #define        std(flags, file) \
        {0,0,0,flags,file,{0,0},0,__sF+file,__sclose,__sread,__sseek,__swrite, \
            {(unsigned char *)(__sFext+file), 0},NULL,0,{0},{0},{0,0},0,0}
 
-                               /* the usual - (stdin + stdout + stderr) */
-static FILE usual[FOPEN_MAX - 3];
-static struct __sfileext usualext[FOPEN_MAX - 3];
-static struct glue uglue = { 0, FOPEN_MAX - 3, usual };
-static struct glue *lastglue = &uglue;
 _THREAD_PRIVATE_MUTEX(__sfp_mutex);
 
-static struct __sfileext __sFext[3];
+static struct __sfileext __sFext[3] = {
+  { { NULL, 0 }, {}, PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, false },
+  { { NULL, 0 }, {}, PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, false },
+  { { NULL, 0 }, {}, PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP, false },
+};
 
 // __sF is exported for backwards compatibility. Until M, we didn't have symbols
 // for stdin/stdout/stderr; they were macros accessing __sF.
 FILE __sF[3] = {
-       std(__SRD, STDIN_FILENO),               /* stdin */
-       std(__SWR, STDOUT_FILENO),              /* stdout */
-       std(__SWR|__SNBF, STDERR_FILENO)        /* stderr */
+  std(__SRD, STDIN_FILENO),
+  std(__SWR, STDOUT_FILENO),
+  std(__SWR|__SNBF, STDERR_FILENO),
 };
 
-struct glue __sglue = { &uglue, 3, __sF };
-
 FILE* stdin = &__sF[0];
 FILE* stdout = &__sF[1];
 FILE* stderr = &__sF[2];
 
+struct glue __sglue = { NULL, 3, __sF };
+static struct glue* lastglue = &__sglue;
+
 static struct glue *
 moreglue(int n)
 {
@@ -114,9 +112,6 @@ __sfp(void)
        int n;
        struct glue *g;
 
-       if (!__sdidinit)
-               __sinit();
-
        _THREAD_PRIVATE_MUTEX_LOCK(__sfp_mutex);
        for (g = &__sglue; g != NULL; g = g->next) {
                for (fp = g->iobs, n = g->niobs; --n >= 0; fp++)
@@ -149,48 +144,7 @@ found:
        return (fp);
 }
 
-/*
- * exit() and abort() call _cleanup() through the callback registered
- * with __atexit_register_cleanup(), set whenever we open or buffer a
- * file. This chicanery is done so that programs that do not use stdio
- * need not link it all in.
- *
- * The name `_cleanup' is, alas, fairly well known outside stdio.
- */
-void
-_cleanup(void)
-{
+__LIBC_HIDDEN__ void __libc_stdio_cleanup(void) {
        /* (void) _fwalk(fclose); */
        (void) _fwalk(__sflush);                /* `cheating' */
 }
-
-/*
- * __sinit() is called whenever stdio's internal variables must be set up.
- */
-void
-__sinit(void)
-{
-       _THREAD_PRIVATE_MUTEX(__sinit_mutex);
-
-       _THREAD_PRIVATE_MUTEX_LOCK(__sinit_mutex);
-       if (__sdidinit) {
-               /* bail out if caller lost the race */
-               _THREAD_PRIVATE_MUTEX_UNLOCK(__sinit_mutex);
-               return;
-       }
-
-       /* Initialize stdin/stdout/stderr (for the recursive mutex). http://b/18208568. */
-       for (size_t i = 0; i < 3; ++i) {
-               _FILEEXT_SETUP(__sF+i, __sFext+i);
-       }
-       /* Initialize the pre-allocated (but initially unused) streams. */
-       for (size_t i = 0; i < FOPEN_MAX - 3; ++i) {
-               _FILEEXT_SETUP(usual+i, usualext+i);
-       }
-
-       /* make sure we clean up on exit */
-       __atexit_register_cleanup(_cleanup); /* conservative */
-       __sdidinit = 1;
-
-       _THREAD_PRIVATE_MUTEX_UNLOCK(__sinit_mutex);
-}
index 3ae7059..6dcd3ae 100644 (file)
@@ -153,10 +153,8 @@ __LIBC32_LEGACY_PUBLIC__ int __srefill(FILE*);
 __LIBC32_LEGACY_PUBLIC__ int __swsetup(FILE*);
 
 /* These were referenced by a couple of different pieces of middleware and the Crystax NDK. */
-__LIBC32_LEGACY_PUBLIC__ extern int __sdidinit;
 __LIBC32_LEGACY_PUBLIC__ int __sflags(const char*, int*);
 __LIBC32_LEGACY_PUBLIC__ FILE* __sfp(void);
-__LIBC32_LEGACY_PUBLIC__ void __sinit(void);
 __LIBC32_LEGACY_PUBLIC__ void __smakebuf(FILE*);
 
 /* These are referenced by the Greed for Glory franchise. */
@@ -170,7 +168,6 @@ __LIBC32_LEGACY_PUBLIC__ int _fwalk(int (*)(FILE *));
 #pragma GCC visibility push(hidden)
 
 int    __sflush_locked(FILE *);
-void   _cleanup(void);
 int    __swhatbuf(FILE *, size_t *, int *);
 wint_t __fgetwc_unlock(FILE *);
 wint_t __ungetwc(wint_t, FILE *);
@@ -179,8 +176,6 @@ int __svfscanf(FILE * __restrict, const char * __restrict, __va_list);
 int    __vfwprintf(FILE * __restrict, const wchar_t * __restrict, __va_list);
 int    __vfwscanf(FILE * __restrict, const wchar_t * __restrict, __va_list);
 
-extern void __atexit_register_cleanup(void (*)(void));
-
 /*
  * Return true if the given FILE cannot be written now.
  */
@@ -237,6 +232,10 @@ struct __suio;
 extern int __sfvwrite(FILE *, struct __suio *);
 wint_t __fputwc_unlock(wchar_t wc, FILE *fp);
 
+/* Remove the if (!__sdidinit) __sinit() idiom from untouched upstream stdio code. */
+extern void __sinit(void); // Not actually implemented.
+#define __sdidinit 1
+
 #pragma GCC visibility pop
 
 __END_DECLS
index e87c7b9..5b0811f 100644 (file)
@@ -51,11 +51,6 @@ lflush(FILE *fp)
 int
 __srefill(FILE *fp)
 {
-
-       /* make sure stdio is set up */
-       if (!__sdidinit)
-               __sinit();
-
        fp->_r = 0;             /* largely a convenience for callers */
 
 #if !defined(__ANDROID__)
index 34a4db1..c817b63 100644 (file)
@@ -185,51 +185,12 @@ restart:
        }
        _ATEXIT_UNLOCK();
 
+  extern void __libc_stdio_cleanup(void);
+  __libc_stdio_cleanup();
+
   /* BEGIN android-changed: call __unregister_atfork if dso is not null */
   if (dso != NULL) {
     __unregister_atfork(dso);
   }
   /* END android-changed */
 }
-
-/*
- * Register the cleanup function
- */
-void
-__atexit_register_cleanup(void (*func)(void))
-{
-       struct atexit *p;
-       size_t pgsize = getpagesize();
-
-       if (pgsize < sizeof(*p))
-               return;
-       _ATEXIT_LOCK();
-       p = __atexit;
-       while (p != NULL && p->next != NULL)
-               p = p->next;
-       if (p == NULL) {
-               p = mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
-                   MAP_ANON | MAP_PRIVATE, -1, 0);
-               if (p == MAP_FAILED)
-                       goto unlock;
-/* BEGIN android-changed */
-               prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, p, pgsize,
-                   "atexit handlers");
-/* END android-changed */
-               p->ind = 1;
-               p->max = (pgsize - ((char *)&p->fns[0] - (char *)p)) /
-                   sizeof(p->fns[0]);
-               p->next = NULL;
-               __atexit = p;
-       } else {
-               if (mprotect(p, pgsize, PROT_READ | PROT_WRITE))
-                       goto unlock;
-       }
-       p->fns[0].fn_ptr = (void (*)(void *))func;
-       p->fns[0].fn_arg = NULL;
-       p->fns[0].fn_dso = NULL;
-       mprotect(p, pgsize, PROT_READ);
-       restartloop = 1;
-unlock:
-       _ATEXIT_UNLOCK();
-}