Re: [Fwd: dlopen regression in 1.7? (or is it just me?)]

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Aug 12 09:36, Corinna Vinschen wrote:
> On Aug 12 02:14, Dave Korn wrote:
> > Corinna Vinschen wrote:
> > 
> >   Yes, this is a real problem.  It's 2am over here; I'll get onto it in the
> > morning.  Sorry for the inconvenience; I really didn't expect people to try to
> > unload their override of a vital library function at runtime and still expect it
> > to work!
> > [...]
> The real ugly problem is that this code is not part of Cygwin, but is
> part of the DLL startup code linked in statically from libcygwin.a.
> That means, every DLL linked against Cygwin-1.7.0-49 and later (since
> back to 2009-06-08), which is potentially dlopened and dlclosed, has to
> be rebuild.
> 
> [...time passes...]
> 
> Of course, we could also kludge dlopen to workaround "broken" DLLs.
> It could store the old cxx_malloc pointer before calling LoadLibrary and 
> restore it afterwards.  This would allow us to stick with these DLLs.
> 
> I tested the below patch to dlopen.  It seems to work nicely for the
> testcase sent by Peter.  Any drawbacks?

Apart from the dlopen patch for now existing, broken DLLs, I think that
the simplest solution is to disallow to override cxx_malloc for
dynamically loaded DLLs.  I tested the below patch with Peter's
testcase, disabling the dlopen hack from my previous mail while doing
it:


	* include/cygwin/cygwin_dll.h (__dynamically_loaded): New global
	variable.
	(_cygwin_dll_entry): Set __dynamically_loaded according to its name.
	* lib/_cygwin_crt0_common.cc (_cygwin_crt0_common): Only override
	C++ memory operators for statically linked DLLs.  Add comment to
	explain why.


Index: include/cygwin/cygwin_dll.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/cygwin_dll.h,v
retrieving revision 1.8
diff -u -p -r1.8 cygwin_dll.h
--- include/cygwin/cygwin_dll.h	11 Sep 2001 20:01:01 -0000	1.8
+++ include/cygwin/cygwin_dll.h	12 Aug 2009 08:42:32 -0000
@@ -33,6 +33,7 @@ CDECL_END								      \
 static HINSTANCE storedHandle;						      \
 static DWORD storedReason;						      \
 static void* storedPtr;							      \
+int __dynamically_loaded;						      \
 									      \
 static int __dllMain (int a, char **b, char **c)			      \
 {									      \
@@ -53,6 +54,7 @@ int WINAPI _cygwin_dll_entry (HINSTANCE 
       storedHandle = h;							      \
       storedReason = reason;						      \
       storedPtr = ptr;							      \
+      __dynamically_loaded = (ptr == NULL);				      \
       dll_index = cygwin_attach_dll (h, &__dllMain);			      \
       if (dll_index == (DWORD) -1)					      \
 	ret = 0;							      \
Index: lib/_cygwin_crt0_common.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/lib/_cygwin_crt0_common.cc,v
retrieving revision 1.17
diff -u -p -r1.17 _cygwin_crt0_common.cc
--- lib/_cygwin_crt0_common.cc	7 Jul 2009 20:12:44 -0000	1.17
+++ lib/_cygwin_crt0_common.cc	12 Aug 2009 08:42:32 -0000
@@ -62,6 +62,8 @@ struct per_process_cxx_malloc __cygwin_c
 /* Set up pointers to various pieces so the dll can then use them,
    and then jump to the dll.  */
 
+extern int __dynamically_loaded;
+
 int __stdcall
 _cygwin_crt0_common (MainFunc f, per_process *u)
 {
@@ -114,27 +116,32 @@ _cygwin_crt0_common (MainFunc f, per_pro
   u->realloc = &realloc;
   u->calloc = &calloc;
 
-  /* Likewise for the C++ memory operators - if any.  */
-  if (newu && newu->cxx_malloc)
+  if (!__dynamically_loaded)
     {
-      /* Inherit what we don't override.  */
-#define CONDITIONALLY_OVERRIDE(MEMBER) \
-      if (!__cygwin_cxx_malloc.MEMBER) \
-	__cygwin_cxx_malloc.MEMBER = newu->cxx_malloc->MEMBER;
-      CONDITIONALLY_OVERRIDE(oper_new);
-      CONDITIONALLY_OVERRIDE(oper_new__);
-      CONDITIONALLY_OVERRIDE(oper_delete);
-      CONDITIONALLY_OVERRIDE(oper_delete__);
-      CONDITIONALLY_OVERRIDE(oper_new_nt);
-      CONDITIONALLY_OVERRIDE(oper_new___nt);
-      CONDITIONALLY_OVERRIDE(oper_delete_nt);
-      CONDITIONALLY_OVERRIDE(oper_delete___nt);
+      /* Likewise for the C++ memory operators - if any. 
+         Only do that for statically linked DLLs, otherwise the cxx_malloc
+	 pointer could potentially point into volatile memory (dlclose). */
+      if (newu && newu->cxx_malloc && !__dynamically_loaded)
+	{
+	  /* Inherit what we don't override.  */
+    #define CONDITIONALLY_OVERRIDE(MEMBER) \
+	  if (!__cygwin_cxx_malloc.MEMBER) \
+	    __cygwin_cxx_malloc.MEMBER = newu->cxx_malloc->MEMBER;
+	  CONDITIONALLY_OVERRIDE(oper_new);
+	  CONDITIONALLY_OVERRIDE(oper_new__);
+	  CONDITIONALLY_OVERRIDE(oper_delete);
+	  CONDITIONALLY_OVERRIDE(oper_delete__);
+	  CONDITIONALLY_OVERRIDE(oper_new_nt);
+	  CONDITIONALLY_OVERRIDE(oper_new___nt);
+	  CONDITIONALLY_OVERRIDE(oper_delete_nt);
+	  CONDITIONALLY_OVERRIDE(oper_delete___nt);
+	}
+
+      /* Now update the resulting set into the global redirectors.  */
+      if (newu)
+	newu->cxx_malloc = &__cygwin_cxx_malloc;
     }
 
-  /* Now update the resulting set into the global redirectors.  */
-  if (newu)
-    newu->cxx_malloc = &__cygwin_cxx_malloc;
-
   /* Setup the module handle so fork can get the path name.  */
   u->hmodule = GetModuleHandle (0);


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat