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

[Dave Korn]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: libstdc-dll-vis-final.diff
Type: text/x-c
Size: 12797 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20091120/11dcf136/attachment.bin>