Re: Libstdc++ as DLL on windows, alternative approach [was Re: cygwin patch review]

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Benjamin Kosnik wrote:

>> However it does sound like the right thing to do, so I've spun a new
>> version of the patch that applies dllimport to the namespace.  We
>> could commit this and then open a PR about dllimport not working on
>> namespaces.  

> I like this plan. What's the PR number? And can you affix your
> patch from here to it:
> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00979.html

  This is now PR 42123.  (The WIP patch is on the right lines but not quite
correct yet, details in the PR.)

> Thanks. 
>  
>>   Initial test results (c and c++ only) are posted(*) and look right,
>> but I'll do a clean run on the same source revision for comparison
>> anyway.  Note that I had to patch various testsuite/lib/ files to
>> avoid the current utf-8 problem(**) that would otherwise have
>> provoked many spurious 'excess errors' warnings.  That'll be the
>> subject of a separate patch, I'll do the clean run using them as well
>> so the comparison is fair.
> 
> It looks like (in later messages) you have an updated patch. 

  I've since made one small change (apart from experimenting with the
namespace patch).  In testing the namespace patch, I discovered an order of
include problem, where this code from include/bits/c++config:

>  #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY
>  # define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (#V)))
> +#elif defined (_GLIBCXX_PSEUDO_VISIBILITY)
> +# define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V)
>  #else
>  # define _GLIBCXX_VISIBILITY_ATTR(V)
>  #endif

is before any of the os/cpu-specific headers get #included, so it didn't work.
 I've rearranged it now so that this site at the top of the file just says

> #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY
> # define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (#V)))
> #else
> // If this is not supplied by the OS-specific or CPU-specific
> // headers included below, it will be defined to an empty default.
> # define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V)
> #endif

.. and then after the #includes I added

> // If platform uses neither visibility nor psuedo-visibility,
> // specify empty default for namespace annotation macros.
> #ifndef _GLIBCXX_PSEUDO_VISIBILITY
> #define _GLIBCXX_PSEUDO_VISIBILITY(V)
> #endif

  I've verified this change DTRT by diffing pre-processed sources compiled
with and without -D_GLIBCXX_DLL.

> But your
> updated results here looks great:
> http://gcc.gnu.org/ml/gcc-testresults/2009-11/msg01084.html

  Yep, the results are now consistently the same as for static libstdc++.

>>   So, is this any more like OK for head?
> 
> Yes, exactly. Thanks. This is OK with me.

  :)  Fantastic.  Three small things which need explicit confirmation:

- can your OK cover the c++-related change to LINK_SPEC in gcc/gcc.c or do I
need to get another maintainer (probably jsm) for that?
- is it OK even with the change to the c++config hunk mentioned above?
- is it OK even in stage 3?

  Also, PING! Danny/Kai, the windows side of this patch has changed a bit
since you guys OK'd it some time back, can one of you take a look over the new
changes to config/i386/winnt.c w.r.t. i386_find_on_wrapper_list() and #ifdef
CXX_WRAP_SPEC_LIST?

  I've just updated my sandbox to head.  Attached, the final version of the
patch for reference; I'm just rebuilding it now to check nothing has broken.
The ChangeLog is almost the same as last time, but mentions the c++config change:

gcc/ChangeLog:

	* configure.ac (USE_CYGWIN_LIBSTDCXX_WRAPPERS): Define to reflect
	status of AC_CHECK_FUNC for Cygwin DLL libstdc++ support wrappers.
	* configure: Regenerate.
	* config.in: Regenerate.

	* config/i386/cygwin.h (CXX_WRAP_SPEC_LIST): Define list of --wrap
	options for Cygwin DLL libstdc++ support wrappers.
	(CXX_WRAP_SPEC_OPT): Define spec to use wrappers or not by default
	according to defined value of USE_CYGWIN_LIBSTDCXX_WRAPPERS.
	(CXX_WRAP_SPEC): Define entire wrapper spec in or out according to
	whether USE_CYGWIN_LIBSTDCXX_WRAPPERS is even defined or not.
	(LINK_SPEC): Include CXX_WRAP_SPEC.
	* gcc/config/i386/winnt.c (wrapper_strcmp): New qsort helper function.
	(i386_find_on_wrapper_list): Check if a function is found on the list
	of libstdc++ wrapper options.
	(i386_pe_file_end): If we are importing a wrapped function, also emit
	an external declaration for the real version.
	* config/i386/cygming.opt (muse-libstdc-wrappers): New option for
	Cygwin targets. Update copyright year.

	* gcc.c (LINK_COMMAND_SPEC): Allow and ignore -static-libstdc++
	similarly to -static.

gcc/cp/ChangeLog:

	* g++spec.c (SKIPOPT): Delete.
	(lang_specific_driver): Do not skip -static-libstdc++ option.

libstdc++-v3/ChangeLog:

	* libstdc++-v3/acinclude.m4 (GLIBCXX_ENABLE_SYMVERS): Don't disable
	on PE targets.
	* libstdc++-v3/configure: Regenerate.
	* libstdc++-v3/configure.host: Add libtool DLL options for Cygwin
	and MinGW platforms.

	* libstdc++-v3/include/bits/c++config (_GLIBCXX_VISIBILITY_ATTR): On
	platforms that don't support visibility, allow them to declare a macro
	_GLIBCXX_PSEUDO_VISIBILITY that is applied in place of visibility.
	(_GLIBCXX_PSEUDO_VISIBILITY): Supply empty default if not declared by
	CPU- or OS-specific headers.

	* libstdc++-v3/config/os/newlib/os_defines.h
	(_GLIBCXX_PSEUDO_VISIBILITY_default): New macro for dllimport.
	(_GLIBCXX_PSEUDO_VISIBILITY_hidden): New empty macro.
	(_GLIBCXX_PSEUDO_VISIBILITY): Evaluate to one of the above.
	* libstdc++-v3/config/os/mingw32/os_defines.h
	(_GLIBCXX_PSEUDO_VISIBILITY_default,
	_GLIBCXX_PSEUDO_VISIBILITY_hidden,
	_GLIBCXX_PSEUDO_VISIBILITY): Likewise.

    cheers,
      DaveK

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 154370)
+++ gcc/gcc.c	(working copy)
@@ -766,7 +766,10 @@ proper position among the other output files.  */
 
 /* -u* was put back because both BSD and SysV seem to support it.  */
 /* %{static:} simply prevents an error message if the target machine
-   doesn't handle -static.  */
+   doesn't handle -static; %{static-libstdc++} is a C++ FE option, 
+   handled entirely in cp/g++spec.c, yet we cannot just remove it
+   from the command-line there or it cannot be tested in target spec
+   processing, so we simply ignore it here similarly.  */
 /* We want %{T*} after %{L*} and %D so that it can be used to specify linker
    scripts which exist in user specified directories, or in standard
    directories.  */
@@ -789,7 +792,7 @@ proper position among the other output files.  */
     %{flto} %{fwhopr} %l " LINK_PIE_SPEC \
    "%X %{o*} %{A} %{d} %{e*} %{m} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{x} %{z} %{Z} %{!A:%{!nostdlib:%{!nostartfiles:%S}}}\
-    %{static:} %{L*} %(mfwrap) %(link_libgcc) %o\
+    %{static:} %{static-libstdc++:} %{L*} %(mfwrap) %(link_libgcc) %o\
     %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)} %(mflib)\
     %{fprofile-arcs|fprofile-generate*|coverage:-lgcov}\
     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
Index: gcc/cp/g++spec.c
===================================================================
--- gcc/cp/g++spec.c	(revision 154370)
+++ gcc/cp/g++spec.c	(working copy)
@@ -30,8 +30,6 @@ along with GCC; see the file COPYING3.  If not see
 #define MATHLIB		(1<<2)
 /* This bit is set if they did `-lc'.  */
 #define WITHLIBC	(1<<3)
-/* Skip this option.  */
-#define SKIPOPT		(1<<4)
 
 #ifndef MATH_LIBRARY
 #define MATH_LIBRARY "-lm"
@@ -211,10 +209,7 @@ lang_specific_driver (int *in_argc, const char *co
 	  else if (strcmp (argv[i], "-static-libgcc") == 0)
 	    shared_libgcc = 0;
 	  else if (strcmp (argv[i], "-static-libstdc++") == 0)
-	    {
 	      library = library >= 0 ? 2 : library;
-	      args[i] |= SKIPOPT;
-	    }
 	  else if (DEFAULT_WORD_SWITCH_TAKES_ARG (&argv[i][1]))
 	    i++;
 	  else
@@ -324,9 +319,6 @@ lang_specific_driver (int *in_argc, const char *co
 	  arglist[j] = "-xnone";
 	}
 
-      if ((args[i] & SKIPOPT) != 0)
-	--j;
-
       i++;
       j++;
     }
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 154370)
+++ gcc/configure.ac	(working copy)
@@ -3000,6 +3000,25 @@ changequote(,)dnl
   i[34567]86-*-* | x86_64-*-*)
 changequote([,])dnl
     case $target_os in
+      cygwin*)
+	# Full C++ conformance when using a shared libstdc++-v3 requires some
+	# support from the Cygwin DLL, which in more recent versions exports
+	# wrappers to aid in interposing and redirecting operators new, delete,
+	# etc., as per n2800 #17.6.4.6 [replacement.functions].  Check if we
+	# are configuring for a version of Cygwin that exports the wrappers.
+	if test x$host = x$target; then
+	  AC_CHECK_FUNC([__wrap__Znaj],[gcc_ac_cygwin_dll_wrappers=yes],[gcc_ac_cygwin_dll_wrappers=no])
+	else
+	  # Can't check presence of libc functions during cross-compile, so
+	  # we just have to assume we're building for an up-to-date target.
+	  gcc_ac_cygwin_dll_wrappers=yes
+	fi
+	AC_DEFINE_UNQUOTED(USE_CYGWIN_LIBSTDCXX_WRAPPERS,
+	  [`if test $gcc_ac_cygwin_dll_wrappers = yes; then echo 1; else echo 0; fi`],
+	  [Define if you want to generate code by default that assumes that the
+	   Cygwin DLL exports wrappers to support libstdc++ function replacement.])
+    esac
+    case $target_os in
       cygwin* | pe | mingw32*)
 	# Recent binutils allows the three-operand form of ".comm" on PE.  This
 	# definition is used unconditionally to initialise the default state of
Index: gcc/config/i386/cygwin.h
===================================================================
--- gcc/config/i386/cygwin.h	(revision 154370)
+++ gcc/config/i386/cygwin.h	(working copy)
@@ -85,9 +85,41 @@ along with GCC; see the file COPYING3.  If not see
   %{mwindows:-lgdi32 -lcomdlg32} \
   -luser32 -lkernel32 -ladvapi32 -lshell32"
 
+/* To implement C++ function replacement we always wrap the cxx
+   malloc-like operators.  See N2800 #17.6.4.6 [replacement.functions] */
+#define CXX_WRAP_SPEC_LIST "%{!static: %{!static-libstdc++: \
+  --wrap _Znwj \
+  --wrap _Znaj \
+  --wrap _ZdlPv \
+  --wrap _ZdaPv \
+  --wrap _ZnwjRKSt9nothrow_t \
+  --wrap _ZnajRKSt9nothrow_t \
+  --wrap _ZdlPvRKSt9nothrow_t \
+  --wrap _ZdaPvRKSt9nothrow_t \
+  }}"
+
+#if defined (USE_CYGWIN_LIBSTDCXX_WRAPPERS)
+
+#if USE_CYGWIN_LIBSTDCXX_WRAPPERS
+/* Default on, only explict -mno disables.  */
+#define CXX_WRAP_SPEC_OPT "!mno-use-libstdc-wrappers"
+#else
+/* Default off, only explict -m enables.  */
+#define CXX_WRAP_SPEC_OPT "muse-libstdc-wrappers"
+#endif
+
+#define CXX_WRAP_SPEC "%{" CXX_WRAP_SPEC_OPT ":" CXX_WRAP_SPEC_LIST "}"
+
+#else /* !defined (USE_CYGWIN_LIBSTDCXX_WRAPPERS)  */
+
+#define CXX_WRAP_SPEC ""
+
+#endif /* ?defined (USE_CYGWIN_LIBSTDCXX_WRAPPERS) */
+
 #define LINK_SPEC "\
   %{mwindows:--subsystem windows} \
   %{mconsole:--subsystem console} \
+  " CXX_WRAP_SPEC " \
   %{shared: %{mdll: %eshared and mdll are not compatible}} \
   %{shared: --shared} %{mdll:--dll} \
   %{static:-Bstatic} %{!static:-Bdynamic} \
Index: gcc/config/i386/winnt.c
===================================================================
--- gcc/config/i386/winnt.c	(revision 154370)
+++ gcc/config/i386/winnt.c	(working copy)
@@ -603,6 +603,64 @@ i386_pe_maybe_record_exported_symbol (tree decl, c
   export_head = p;
 }
 
+#ifdef CXX_WRAP_SPEC_LIST
+
+/*  Hash table equality helper function.  */
+
+static int
+wrapper_strcmp (const void *x, const void *y)
+{
+  return !strcmp ((const char *) x, (const char *) y);
+}
+
+/* Search for a function named TARGET in the list of library wrappers
+   we are using, returning a pointer to it if found or NULL if not.
+   This function might be called on quite a few symbols, and we only
+   have the list of names of wrapped functions available to us as a
+   spec string, so first time round we lazily initialise a hash table
+   to make things quicker.  */
+
+static const char *
+i386_find_on_wrapper_list (const char *target)
+{
+  static char first_time = 1;
+  static htab_t wrappers;
+
+  if (first_time)
+    {
+      /* Beware that this is not a complicated parser, it assumes
+         that any sequence of non-whitespace beginning with an
+	 underscore is one of the wrapped symbols.  For now that's
+	 adequate to distinguish symbols from spec substitutions
+	 and command-line options.  */
+      static char wrapper_list_buffer[] = CXX_WRAP_SPEC_LIST;
+      char *bufptr;
+      /* Breaks up the char array into separated strings
+         strings and enter them into the hash table.  */
+      wrappers = htab_create_alloc (8, htab_hash_string, wrapper_strcmp,
+	0, xcalloc, free);
+      for (bufptr = wrapper_list_buffer; *bufptr; ++bufptr)
+	{
+	  char *found = NULL;
+	  if (ISSPACE (*bufptr))
+	    continue;
+	  if (*bufptr == '_')
+	    found = bufptr;
+	  while (*bufptr && !ISSPACE (*bufptr))
+	    ++bufptr;
+	  if (*bufptr)
+	    *bufptr = 0;
+	  if (found)
+	    *htab_find_slot (wrappers, found, INSERT) = found;
+	}
+      first_time = 0;
+    }
+
+  return (const char *) htab_find (wrappers, target);
+}
+
+#endif /* CXX_WRAP_SPEC_LIST */
+
 /* This is called at the end of assembly.  For each external function
    which has not been defined, we output a declaration now.  We also
    output the .drectve section.  */
@@ -624,6 +682,15 @@ i386_pe_file_end (void)
       if (! TREE_ASM_WRITTEN (decl)
 	  && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
 	{
+#ifdef CXX_WRAP_SPEC_LIST
+	  /* To ensure the DLL that provides the corresponding real
+	     functions is still loaded at runtime, we must reference
+	     the real function so that an (unused) import is created.  */
+	  const char *realsym = i386_find_on_wrapper_list (p->name);
+	  if (realsym)
+	    i386_pe_declare_function_type (asm_out_file,
+		concat ("__real_", realsym, NULL), TREE_PUBLIC (decl));
+#endif /* CXX_WRAP_SPEC_LIST */
 	  TREE_ASM_WRITTEN (decl) = 1;
 	  i386_pe_declare_function_type (asm_out_file, p->name,
 					 TREE_PUBLIC (decl));
Index: gcc/config/i386/cygming.opt
===================================================================
--- gcc/config/i386/cygming.opt	(revision 154370)
+++ gcc/config/i386/cygming.opt	(working copy)
@@ -1,6 +1,6 @@
 ; Cygwin- and MinGW-specific options.
 
-; Copyright (C) 2005, 2007 Free Software Foundation, Inc.
+; Copyright (C) 2005, 2007, 2009 Free Software Foundation, Inc.
 ;
 ; This file is part of GCC.
 ;
@@ -49,3 +49,7 @@ Create GUI application
 mpe-aligned-commons
 Target Var(use_pe_aligned_common) Init(HAVE_GAS_ALIGNED_COMM)
 Use the GNU extension to the PE format for aligned common data
+
+muse-libstdc-wrappers
+Target Condition({defined (USE_CYGWIN_LIBSTDCXX_WRAPPERS)})
+Compile code that relies on Cygwin DLL wrappers to support C++ operator new/delete replacement
Index: libstdc++-v3/configure.host
===================================================================
--- libstdc++-v3/configure.host	(revision 154370)
+++ libstdc++-v3/configure.host	(working copy)
@@ -209,6 +209,7 @@ case "${host_os}" in
     ;;
   cygwin*)
     os_include_dir="os/newlib"
+    OPT_LDFLAGS="${OPT_LDFLAGS} -no-undefined -bindir \$(bindir)"
     ;;
   darwin | darwin[1-7] | darwin[1-7].*)
     # On Darwin, performance is improved if libstdc++ is single-module.
@@ -258,6 +259,7 @@ case "${host_os}" in
   mingw32*)
     os_include_dir="os/mingw32"
     error_constants_dir="os/mingw32"
+    OPT_LDFLAGS="${OPT_LDFLAGS} -no-undefined -bindir \$(bindir)"
     ;;
   netbsd*)
     os_include_dir="os/bsd/netbsd"
Index: libstdc++-v3/include/bits/c++config
===================================================================
--- libstdc++-v3/include/bits/c++config	(revision 154370)
+++ libstdc++-v3/include/bits/c++config	(working copy)
@@ -42,7 +42,9 @@
 #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY
 # define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (#V)))
 #else
-# define _GLIBCXX_VISIBILITY_ATTR(V) 
+// If this is not supplied by the OS-specific or CPU-specific
+// headers included below, it will be defined to an empty default.
+# define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V)
 #endif
 
 // Macros for deprecated.
@@ -275,6 +277,12 @@ namespace std
 // Pick up any CPU-specific definitions.
 #include <bits/cpu_defines.h>
 
+// If platform uses neither visibility nor psuedo-visibility,
+// specify empty default for namespace annotation macros.
+#ifndef _GLIBCXX_PSEUDO_VISIBILITY
+#define _GLIBCXX_PSEUDO_VISIBILITY(V)
+#endif
+
 // Allow use of "export template." This is currently not a feature
 // that g++ supports.
 // #define _GLIBCXX_EXPORT_TEMPLATE 1
Index: libstdc++-v3/config/os/newlib/os_defines.h
===================================================================
--- libstdc++-v3/config/os/newlib/os_defines.h	(revision 154370)
+++ libstdc++-v3/config/os/newlib/os_defines.h	(working copy)
@@ -36,6 +36,15 @@
 #ifdef __CYGWIN__
 #define _GLIBCXX_GTHREAD_USE_WEAK 0
 
+#if defined (_GLIBCXX_DLL)
+#define _GLIBCXX_PSEUDO_VISIBILITY_default __attribute__ ((__dllimport__))
+#else
+#define _GLIBCXX_PSEUDO_VISIBILITY_default
+#endif
+#define _GLIBCXX_PSEUDO_VISIBILITY_hidden
+
+#define _GLIBCXX_PSEUDO_VISIBILITY(V) _GLIBCXX_PSEUDO_VISIBILITY_ ## V
+
 // See libstdc++/20806.
 #define _GLIBCXX_HAVE_DOS_BASED_FILESYSTEM 1
 #endif
Index: libstdc++-v3/config/os/mingw32/os_defines.h
===================================================================
--- libstdc++-v3/config/os/mingw32/os_defines.h	(revision 154370)
+++ libstdc++-v3/config/os/mingw32/os_defines.h	(working copy)
@@ -45,6 +45,15 @@
 #undef NOMINMAX
 #define NOMINMAX 1
 
+#if defined (_GLIBCXX_DLL)
+#define _GLIBCXX_PSEUDO_VISIBILITY_default __attribute__ ((__dllimport__))
+#else
+#define _GLIBCXX_PSEUDO_VISIBILITY_default
+#endif
+#define _GLIBCXX_PSEUDO_VISIBILITY_hidden
+
+#define _GLIBCXX_PSEUDO_VISIBILITY(V) _GLIBCXX_PSEUDO_VISIBILITY_ ## V
+
 // See libstdc++/20806.
 #define _GLIBCXX_HAVE_DOS_BASED_FILESYSTEM 1
 
Index: libstdc++-v3/acinclude.m4
===================================================================
--- libstdc++-v3/acinclude.m4	(revision 154370)
+++ libstdc++-v3/acinclude.m4	(working copy)
@@ -2737,7 +2737,7 @@ if test x$enable_symvers = xyes ; then
   else
     if test $with_gnu_ld = yes ; then
       case ${target_os} in
-        cygwin* | pe | mingw32* | hpux*)
+        hpux*)
           enable_symvers=no ;;
         *)
           enable_symvers=gnu ;;