From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1323 invoked by alias); 20 Nov 2009 16:44:30 -0000 Received: (qmail 1299 invoked by uid 22791); 20 Nov 2009 16:44:25 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_05,KAM_STOCKGEN,SPF_PASS X-Spam-Check-By: sourceware.org Received: from ey-out-1920.google.com (HELO ey-out-1920.google.com) (74.125.78.148) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 20 Nov 2009 16:43:44 +0000 Received: by ey-out-1920.google.com with SMTP id 13so924545eye.46 for ; Fri, 20 Nov 2009 08:43:41 -0800 (PST) Received: by 10.213.24.12 with SMTP id t12mr1521191ebb.70.1258735421707; Fri, 20 Nov 2009 08:43:41 -0800 (PST) Received: from ?192.168.2.99? (cpc2-cmbg8-0-0-cust61.cmbg.cable.ntl.com [82.6.108.62]) by mx.google.com with ESMTPS id 7sm419032eyb.18.2009.11.20.08.43.38 (version=SSLv3 cipher=RC4-MD5); Fri, 20 Nov 2009 08:43:40 -0800 (PST) Message-ID: <4B06CAEA.5080705@gmail.com> Date: Fri, 20 Nov 2009 16:56:00 -0000 From: Dave Korn User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Benjamin Kosnik CC: Dave Korn , libstdc++@gcc.gnu.org, GCC Patches , Danny Smith , "Aaron W. LaFramboise (GCC)" , Kai Tietz Subject: Re: Libstdc++ as DLL on windows, alternative approach [was Re: cygwin patch review] References: <20091019092452.2e271791@mcgee.artheist.org> <4AFC8EC3.10502@gmail.com> <20091119093347.797df3ff@mcgee.artheist.org> In-Reply-To: <20091119093347.797df3ff@mcgee.artheist.org> Content-Type: multipart/mixed; boundary="------------030102070404070702000101" X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2009-11/txt/msg01108.txt.bz2 This is a multi-part message in MIME format. --------------030102070404070702000101 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 5662 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 --------------030102070404070702000101 Content-Type: text/x-c; name="libstdc-dll-vis-final.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="libstdc-dll-vis-final.diff" Content-length: 12797 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 +// 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 ;; --------------030102070404070702000101--