Re: [RFC] Cygwin libstdc++ plan (operator new/delete replacement)

[Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>]

On Sun, Jun 28, 2009 at 02:47:24PM +0100, Dave Korn wrote:
>to the linker spec.  If we agree this is a reasonable approach to take, I
>could rush out a gcc4-4.3.2-3 update to add this very quickly.
>
>  So, would everyone be happy to do it this way?

It looks ok with a couple of caveats/questions below.

Would it make sense to standardize on this approach for the malloc
functions too and remove the malloc_wrapper file?

>Index: winsup/cygwin/cygwin.din
>===================================================================
>RCS file: /cvs/src/src/winsup/cygwin/cygwin.din,v
>retrieving revision 1.211
>diff -p -u -r1.211 cygwin.din
>--- winsup/cygwin/cygwin.din	31 Mar 2009 09:42:58 -0000	1.211
>+++ winsup/cygwin/cygwin.din	28 Jun 2009 13:15:17 -0000
>@@ -1806,3 +1806,11 @@ y1 NOSIGFE
> y1f NOSIGFE
> yn NOSIGFE
> ynf NOSIGFE
>+__wrap__ZdaPv NOSIGFE
>+__wrap__ZdaPvRKSt9nothrow_t NOSIGFE
>+__wrap__ZdlPv NOSIGFE
>+__wrap__ZdlPvRKSt9nothrow_t NOSIGFE
>+__wrap__Znaj NOSIGFE
>+__wrap__ZnajRKSt9nothrow_t NOSIGFE
>+__wrap__Znwj NOSIGFE
>+__wrap__ZnwjRKSt9nothrow_t NOSIGFE

Could we add comments whereever these mangled functions are referenced
showing what they really refer to when they are demangled?

>Index: winsup/cygwin/globals.cc
>===================================================================
>RCS file: /cvs/src/src/winsup/cygwin/globals.cc,v
>retrieving revision 1.2
>diff -p -u -r1.2 globals.cc
>--- winsup/cygwin/globals.cc	24 Mar 2009 12:18:34 -0000	1.2
>+++ winsup/cygwin/globals.cc	28 Jun 2009 13:15:17 -0000
>@@ -97,7 +97,8 @@ extern "C"
>    /* calloc */ calloc,
>    /* premain */ {NULL, NULL, NULL, NULL},
>    /* run_ctors_p */ 0,
>-   /* unused */ {0, 0, 0, 0, 0, 0, 0},
>+   /* unused */ {0, 0, 0, 0, 0, 0},
>+   /* cxx_malloc */ &default_cygwin_cxx_malloc,

See below.

>Index: winsup/cygwin/include/sys/cygwin.h
>===================================================================
>RCS file: /cvs/src/src/winsup/cygwin/include/sys/cygwin.h,v
>retrieving revision 1.79
>diff -p -u -r1.79 cygwin.h
>--- winsup/cygwin/include/sys/cygwin.h	16 Jan 2009 12:17:28 -0000	1.79
>+++ winsup/cygwin/include/sys/cygwin.h	28 Jun 2009 13:15:17 -0000
>@@ -194,6 +194,8 @@ enum
> class MTinterface;
> #endif
> 
>+struct per_process_cxx_malloc;
>+
> struct per_process
> {
>   char *initial_sp;
>@@ -236,7 +238,8 @@ struct per_process
>   /* non-zero of ctors have been run.  Inherited from parent. */
>   int run_ctors_p;
> 
>-  DWORD unused[7];
>+  DWORD unused[6];
>+  struct per_process_cxx_malloc *cxx_malloc;

This should go before the unused rather than after.
 
>   /* Non-zero means the task was forked.  The value is the pid.
>      Inherited from parent. */
>Index: winsup/cygwin/lib/_cygwin_crt0_common.cc
>===================================================================
>RCS file: /cvs/src/src/winsup/cygwin/lib/_cygwin_crt0_common.cc,v
>retrieving revision 1.16
>diff -p -u -r1.16 _cygwin_crt0_common.cc
>--- winsup/cygwin/lib/_cygwin_crt0_common.cc	3 Jan 2009 05:12:22 -0000	1.16
>+++ winsup/cygwin/lib/_cygwin_crt0_common.cc	28 Jun 2009 13:15:17 -0000
>@@ -10,6 +10,23 @@ details. */
> 
> #include "winsup.h"
> #include "crt0.h"
>+#include "cygwin-cxx.h"
>+
>+// Weaken these declarations so the references don't pull in C++ dependencies unnecessarily.
>+#define WEAK __attribute__ ((weak))
>+
>+// Use asm names to bypass the --wrap that is being applied to redirect all other
>+// references to these operators toward the redirectors in the Cygwin DLL; this
>+// way we can record what definitions were visible at final link time but still
>+// send all calls to the redirectors.
>+extern WEAK void *operator new(std::size_t sz) throw (std::bad_alloc) __asm__ ("___real__Znwj");
>+extern WEAK void *operator new[](std::size_t sz) throw (std::bad_alloc) __asm__ ("___real__Znaj");
>+extern WEAK void operator delete(void *p) throw() __asm__ ("___real__ZdlPv ");
>+extern WEAK void operator delete[](void *p) throw() __asm__ ("___real__ZdaPv");
>+extern WEAK void* operator new(std::size_t sz, const std::nothrow_t &nt) throw() __asm__ ("___real__ZnwjRKSt9nothrow_t");
>+extern WEAK void* operator new[](std::size_t sz, const std::nothrow_t &nt) throw() __asm__ ("___real__ZnajRKSt9nothrow_t");
>+extern WEAK void operator delete(void *p, const std::nothrow_t &nt) throw() __asm__ ("___real__ZdlPvRKSt9nothrow_t");
>+extern WEAK void operator delete[](void *p, const std::nothrow_t &nt) throw() __asm__ ("___real__ZdaPvRKSt9nothrow_t");
> 
> /* Avoid an info message from linker when linking applications. */
> extern __declspec(dllimport) struct _reent *_impure_ptr;
>@@ -25,6 +42,14 @@ int main (int, char **, char **);
> int _fmode;
> void _pei386_runtime_relocator ();
> 
>+struct per_process_cxx_malloc __cygwin_cxx_malloc = 
>+{
>+  &(operator new), &(operator new[]),
>+  &(operator delete), &(operator delete[]),
>+  &(operator new), &(operator new[]),
>+  &(operator delete), &(operator delete[])
>+};
>+
> /* Set up pointers to various pieces so the dll can then use them,
>    and then jump to the dll.  */
> 
>@@ -33,16 +58,16 @@ _cygwin_crt0_common (MainFunc f, per_pro
> {
>   /* This is used to record what the initial sp was.  The value is needed
>      when copying the parent's stack to the child during a fork.  */
>-  DWORD newu;
>+  per_process *newu = (per_process *) cygwin_internal (CW_USER_DATA);
>   int uwasnull;
> 
>   if (u != NULL)
>     uwasnull = 0;	/* Caller allocated space for per_process structure */
>-  else if ((newu = cygwin_internal (CW_USER_DATA)) == (DWORD) -1)
>+  else if (~0u == (DWORD) newu)

I really detest the 0 == variable usage.  I know why people use it (yes,
I really do) but I'd rather not see it in Cygwin source.

But, the good news is that you don't really need it since we're getting
rid of old Cygwin cruft anyway so this should be gone for 1.7.

>@@ -84,6 +109,27 @@ _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)
>+    {
>+      /* 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);
>+    }

I don't suppose there's any way that the above could be in some sort
of array so that the array could just be extended and new functions
handled automatically?

cgf