* cygheap.cc (init_cygheap::find_tls): Add comment.

	* cygtls.cc (well_known_dlls): Rephrase comment.
	(bloda_detect): New function.
	(_cygtls::call2): Call init_thread and bloda_detect for non-pthread
	threads only.
	(_cygtls::remove): Move remove_tls and remove_wq calls up to run first.
	* miscfuncs.cc (struct pthread_wrapper_arg): Rename from struct
	thread_wrapper_arg.
	(pthread_wrapper): Rename from thread_wrapper and drop "static".  Fix
	comment.  Drop call to _cygtls::remove.  Call api_fatal rather than
	ExitThread.  Explain why.
	* miscfuncs.h (pthread_wrapper): Declare pthread_wrapper.
	* thread.cc (pthread::exit): Add a FIXME comment.  Call _cygtls::remove
	before calling ExitThread.

6 files changed, 88 insertions, 57 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index edafd57fa..3eb802f39 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,20 @@
+2014-11-28  Corinna Vinschen  <corinna@vinschen.de>
+
+	* cygheap.cc (init_cygheap::find_tls): Add comment.
+	* cygtls.cc (well_known_dlls): Rephrase comment.
+	(bloda_detect): New function.
+	(_cygtls::call2): Call init_thread and bloda_detect for non-pthread
+	threads only.
+	(_cygtls::remove): Move remove_tls and remove_wq calls up to run first.
+	* miscfuncs.cc (struct pthread_wrapper_arg): Rename from struct
+	thread_wrapper_arg.
+	(pthread_wrapper): Rename from thread_wrapper and drop "static".  Fix
+	comment.  Drop call to _cygtls::remove.  Call api_fatal rather than
+	ExitThread.  Explain why.
+	* miscfuncs.h (pthread_wrapper): Declare pthread_wrapper.
+	* thread.cc (pthread::exit): Add a FIXME comment.  Call _cygtls::remove
+	before calling ExitThread.
+
 2014-11-27  Corinna Vinschen  <corinna@vinschen.de>
 
 	* mount.cc (mount_info::init): Take bool argument and allow to
diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc
index 9ac303eed..594f53c9c 100644
--- a/winsup/cygwin/cygheap.cc
+++ b/winsup/cygwin/cygheap.cc
@@ -673,6 +673,7 @@ init_cygheap::find_tls (int sig, bool& issig_wait)
     {
       __try
 	{
+	    /* Only pthreads have tid set to non-0. */
 	    if (!threadlist[ix]->tid)
 	      continue;
 	    else if (sigismember (&(threadlist[ix]->sigwait_mask), sig))
diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index 2eaae9437..83a62f1ae 100644
--- a/winsup/cygwin/cygtls.cc
+++ b/winsup/cygwin/cygtls.cc
@@ -1,7 +1,7 @@
 /* cygtls.cc
 
    Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012,
-   2013 Red Hat, Inc.
+   2013, 2014 Red Hat, Inc.
 
 This software is a copyrighted work licensed under the terms of the
 Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
@@ -38,10 +38,9 @@ dll_cmp (const void *a, const void *b)
 
 /* Keep sorted!
    This is a list of well-known core system DLLs which contain code
-   whiuch is started in its own thread by the system.  Kernel32.dll,
-   for instance, contains the thread called on every Ctrl-C keypress
-   in a console window.  The DLLs in this list are not recognized as
-   BLODAs. */
+   started in its own thread by the system.  Kernel32.dll, for instance,
+   contains the thread called on every Ctrl-C keypress in a console window.
+   The DLLs in this list are not recognized as BLODAs. */
 const wchar_t *well_known_dlls[] =
 {
   L"advapi32.dll",
@@ -55,46 +54,58 @@ const wchar_t *well_known_dlls[] =
   L"ws2_32.dll",
 };
 
+/* Optional BLODA detection.  The idea is that the function address is supposed
+   to be within Cygwin itself.  This is also true for pthreads, since pthreads
+   are always calling pthread::thread_init_wrapper() in thread.cc.  Therefore,
+   every function call to a function outside of the Cygwin DLL is potentially
+   a thread injected into the Cygwin process by some BLODA.
+
+   But that's too simple.  Assuming the application itself calls CreateThread,
+   then this is a bad idea, but not really invalid.  So we shouldn't print a
+   BLODA message if the address is within the loaded image of the application.
+   Also, ntdll.dll starts threads into the application which */
+static void
+bloda_detect (DWORD (*func) (void *, void *))
+{
+  PIMAGE_DOS_HEADER img_start = (PIMAGE_DOS_HEADER)
+				GetModuleHandle (NULL);
+  PIMAGE_NT_HEADERS32 ntheader = (PIMAGE_NT_HEADERS32)
+			   ((PBYTE) img_start + img_start->e_lfanew);
+  void *img_end = (void *) ((PBYTE) img_start
+			    + ntheader->OptionalHeader.SizeOfImage);
+  if (((void *) func < (void *) cygwin_hmodule
+       || (void *) func > (void *) cygheap)
+      && ((void *) func < (void *) img_start || (void *) func >= img_end))
+    {
+      MEMORY_BASIC_INFORMATION mbi;
+      wchar_t modname[PATH_MAX];
+
+      VirtualQuery ((PVOID) func, &mbi, sizeof mbi);
+      GetModuleFileNameW ((HMODULE) mbi.AllocationBase, modname,
+			  PATH_MAX);
+      /* Fetch basename and check against list of above system DLLs. */
+      const wchar_t *modbasename = wcsrchr (modname, L'\\') + 1;
+      if (!bsearch (modbasename, well_known_dlls,
+		    sizeof well_known_dlls / sizeof well_known_dlls[0],
+		    sizeof well_known_dlls[0], dll_cmp))
+	small_printf ("\n\nPotential BLODA detected!  Thread function "
+		      "called outside of Cygwin DLL:\n  %W\n\n",
+		      modname);
+    }
+}
+
 void
 _cygtls::call2 (DWORD (*func) (void *, void *), void *arg, void *buf)
 {
-  init_thread (buf, func);
-
-  /* Optional BLODA detection.  The idea is that the function address is
-     supposed to be within Cygwin itself.  This is also true for pthreads,
-     since pthreads are always calling thread_wrapper in miscfuncs.cc.
-     Therefore, every function call to a function outside of the Cygwin DLL
-     is potentially a thread injected into the Cygwin process by some BLODA.
-
-     But that's a bit too simple.  Assuming the application itself calls
-     CreateThread, then this is a bad idea, but not really invalid.  So we
-     shouldn't print a BLODA message if the address is within the loaded
-     image of the application.  Also, ntdll.dll starts threads into the
-     application which */
-  if (detect_bloda)
+  /* If func is pthread_wrapper, the final stack hasn't been set up yet.
+     This only happens in pthread_wrapper itself.  Thus it doesn't make
+     sense to call init_thread or perform BLODA detection.  pthread_wrapper
+     eventually calls init_thread by itself. */
+  if ((void *) func != (void *) pthread_wrapper)
     {
-      PIMAGE_DOS_HEADER img_start = (PIMAGE_DOS_HEADER) GetModuleHandle (NULL);
-      PIMAGE_NT_HEADERS32 ntheader = (PIMAGE_NT_HEADERS32)
-			       ((PBYTE) img_start + img_start->e_lfanew);
-      void *img_end = (void *) ((PBYTE) img_start
-				+ ntheader->OptionalHeader.SizeOfImage);
-      if (((void *) func < (void *) cygwin_hmodule
-	   || (void *) func > (void *) cygheap)
-	  && ((void *) func < (void *) img_start || (void *) func >= img_end))
-	{
-	  MEMORY_BASIC_INFORMATION mbi;
-	  wchar_t modname[PATH_MAX];
-
-	  VirtualQuery ((PVOID) func, &mbi, sizeof mbi);
-	  GetModuleFileNameW ((HMODULE) mbi.AllocationBase, modname, PATH_MAX);
-	  /* Fetch basename and check against list of above system DLLs. */
-	  const wchar_t *modbasename = wcsrchr (modname, L'\\') + 1;
-	  if (!bsearch (modbasename, well_known_dlls,
-			sizeof well_known_dlls / sizeof well_known_dlls[0],
-			sizeof well_known_dlls[0], dll_cmp))
-	    small_printf ("\n\nPotential BLODA detected!  Thread function "
-			  "called outside of Cygwin DLL:\n  %W\n\n", modname);
-	}
+      init_thread (buf, func);
+      if (detect_bloda)
+	bloda_detect (func);
     }
 
   DWORD res = func (arg, buf);
@@ -169,6 +180,9 @@ _cygtls::remove (DWORD wait)
 
   debug_printf ("wait %u", wait);
 
+  cygheap->remove_tls (this, INFINITE);
+  remove_wq (wait);
+
   /* FIXME: Need some sort of atthreadexit function to allow things like
      select to control this themselves. */
 
@@ -197,8 +211,6 @@ _cygtls::remove (DWORD wait)
   /* Close timer handle. */
   if (locals.cw_timer)
     NtClose (locals.cw_timer);
-  cygheap->remove_tls (this, wait);
-  remove_wq (wait);
 }
 
 #ifdef __x86_64__
diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc
index eee47a53a..e6ea5cc2d 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -540,7 +540,7 @@ __import_address (void *imp)
    parameters we don't use and instead to add parameters we need to make
    the function pthreads compatible. */
 
-struct thread_wrapper_arg
+struct pthread_wrapper_arg
 {
   LPTHREAD_START_ROUTINE func;
   PVOID arg;
@@ -549,23 +549,20 @@ struct thread_wrapper_arg
   PBYTE stacklimit;
 };
 
-static DWORD WINAPI
-thread_wrapper (PVOID arg)
+DWORD WINAPI
+pthread_wrapper (PVOID arg)
 {
   /* Just plain paranoia. */
   if (!arg)
     return ERROR_INVALID_PARAMETER;
 
-  /* The process is now threaded.  Note the fact for later usage. */
+  /* The process is now threaded.  Note for later usage by arc4random. */
   __isthreaded = 1;
 
   /* Fetch thread wrapper info and free from cygheap. */
-  thread_wrapper_arg wrapper_arg = *(thread_wrapper_arg *) arg;
+  pthread_wrapper_arg wrapper_arg = *(pthread_wrapper_arg *) arg;
   cfree (arg);
 
-  /* Remove _cygtls from this stack since it won't be used anymore. */
-  _my_tls.remove (0);
-
   /* Set stack values in TEB */
   PTEB teb = NtCurrentTeb ();
   teb->Tib.StackBase = wrapper_arg.stackbase;
@@ -676,8 +673,9 @@ thread_wrapper (PVOID arg)
 	   : : [WRAPPER_ARG] "r" (&wrapper_arg),
 	       [CYGTLS] "i" (CYGTLS_PADSIZE));
 #endif
-  /* Never return from here. */
-  ExitThread (0);
+  /* pthread::thread_init_wrapper calls pthread::exit, which
+     in turn calls ExitThread, so we should never arrive here. */
+  api_fatal ("Dumb thinko in pthread handling.  Whip the developer.");
 }
 
 #ifdef __x86_64__
@@ -751,10 +749,10 @@ CygwinCreateThread (LPTHREAD_START_ROUTINE thread_func, PVOID thread_arg,
   PVOID real_stackaddr = NULL;
   ULONG real_stacksize = 0;
   ULONG real_guardsize = 0;
-  thread_wrapper_arg *wrapper_arg;
+  pthread_wrapper_arg *wrapper_arg;
   HANDLE thread = NULL;
 
-  wrapper_arg = (thread_wrapper_arg *) ccalloc (HEAP_STR, 1,
+  wrapper_arg = (pthread_wrapper_arg *) ccalloc (HEAP_STR, 1,
 						sizeof *wrapper_arg);
   if (!wrapper_arg)
     {
@@ -846,7 +844,7 @@ CygwinCreateThread (LPTHREAD_START_ROUTINE thread_func, PVOID thread_arg,
      reserve a 256K stack, not 64K, otherwise the thread creation might
      crash the process due to a stack overflow. */
   thread = CreateThread (&sec_none_nih, 4 * PTHREAD_STACK_MIN,
-			 thread_wrapper, wrapper_arg,
+			 pthread_wrapper, wrapper_arg,
 			 creation_flags | STACK_SIZE_PARAM_IS_A_RESERVATION,
 			 thread_id);
 
diff --git a/winsup/cygwin/miscfuncs.h b/winsup/cygwin/miscfuncs.h
index e42940c1a..c53a520c7 100644
--- a/winsup/cygwin/miscfuncs.h
+++ b/winsup/cygwin/miscfuncs.h
@@ -66,6 +66,7 @@ ssize_t __reg3 check_iovec (const struct iovec *, int, bool);
 #define check_iovec_for_read(a, b) check_iovec ((a), (b), false)
 #define check_iovec_for_write(a, b) check_iovec ((a), (b), true)
 
+extern "C" DWORD WINAPI pthread_wrapper (PVOID arg);
 extern "C" HANDLE WINAPI CygwinCreateThread (LPTHREAD_START_ROUTINE thread_func,
 					     PVOID thread_arg, PVOID stackaddr,
 					     ULONG stacksize, ULONG guardsize,
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index f801d5962..a2e2aeb27 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -543,11 +543,13 @@ pthread::exit (void *value_ptr)
     {
       if (is_main_tls)
 	{
+	  /* FIXME: Needs locking. */
 	  _cygtls *dummy = (_cygtls *) malloc (sizeof (_cygtls));
 	  *dummy = *_main_tls;
 	  _main_tls = dummy;
-	  _main_tls->initialized = false;
+	  _main_tls->initialized = 0;
 	}
+      cygtls->remove (INFINITE);
       ExitThread (0);
     }
 }