From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8614 invoked by alias); 19 Nov 2009 00:33:52 -0000 Received: (qmail 8587 invoked by uid 22791); 19 Nov 2009 00:33:49 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,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.145) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 19 Nov 2009 00:32:41 +0000 Received: by ey-out-1920.google.com with SMTP id 3so2697321eyh.0 for ; Wed, 18 Nov 2009 16:32:38 -0800 (PST) Received: by 10.213.102.202 with SMTP id h10mr736430ebo.85.1258590758876; Wed, 18 Nov 2009 16:32:38 -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 5sm6460eyh.8.2009.11.18.16.32.35 (version=SSLv3 cipher=RC4-MD5); Wed, 18 Nov 2009 16:32:36 -0800 (PST) Message-ID: <4B0495D0.5060609@gmail.com> Date: Thu, 19 Nov 2009 01:25:00 -0000 From: Dave Korn User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Dave Korn CC: Benjamin Kosnik , 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> In-Reply-To: <4AFC8EC3.10502@gmail.com> Content-Type: multipart/mixed; boundary="------------070505020000090202030603" 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/msg00980.txt.bz2 This is a multi-part message in MIME format. --------------070505020000090202030603 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 2928 Dave Korn wrote: > Benjamin Kosnik wrote: >> From older mail thread: >> No. There is already visibility markup on libstdc++ headers for this. >> You can just put the attribute visibility bits on namespace std, and be >> done with it. >> >> Here, from include/bits/c++config.h: >> >> # define _GLIBCXX_BEGIN_NAMESPACE(X) namespace X >> _GLIBCXX_VISIBILITY_ATTR(default) > > Ok, so as we discussed before, this doesn't currently work. 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. It's much smaller and cleaner than > the version that adds markup everywhere, but I doubt I'll be able to fix the > dllimport-vs-namespace bug in time for 4.5. That's not the end of the world; > getting dllimport right is a good optimisation, but we can live with the > compromise of auto-import for now. I've done a little bit of scoping out what it would take to enable dllimport/dllexport attributes on namespaces, and I think it looks roughly something like the attached crude (contains debug printf and commented-out bits) WIP patch. There are some small tweaks to the C++ parser and core attribute handler to allow the attributes to be attached to namespace_decl nodes (probably more than needed in fact, I think rather than passing in a tree* pointer to handle_namespace_attrs in cp/name-lookup.c I just want to gcc_assert that cplus_decl_attributes doesn't return a changed pointer; it shouldn't anyway, it's only meant to do so for types, not decls), and then in the backend we process class definitions and other decls using the adjust_class_at_definition and encode_section_info flags, walking our way up the namespace/class context hierarchy and looking for the attributes. I'm going to put this through bootstrap and test, and then do some more manual testing. I'm not sure whether it handles everything correctly w.r.t. templates and odd corner cases of nested contexts and would be glad of any guidance anyone can offer about the algorithm for walking up out of the nested scopes looking for dll{im,ex}port attributes. Another open question is whether for the sake of this usage (tagging dll attributes on namespaces) it wouldn't make sense to add nodllimport and nodllexport tags, which would serve to block off portions of the hierarchy (i.e. if the scope tree climb ascends past a node tagged with one of these first, it stops early). This would go in the places where ELF platforms use hidden visibility attributes. Anyway, FYI and FTR, this is what I'm trying at the moment. Doesn't look like there's anything we could do solely in the backend to try and make it suitable for stage 3, which I had kinda hoped, but the attributes just get discarded in the parser without the core compiler mods; shame. cheers, DaveK --------------070505020000090202030603 Content-Type: text/x-c; name="dllimport-namespace-wip.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="dllimport-namespace-wip.diff" Content-length: 11729 Index: gcc/tree.c =================================================================== --- gcc/tree.c (revision 154010) +++ gcc/tree.c (working copy) @@ -5307,7 +5307,8 @@ handle_dll_attribute (tree * pnode, tree name, tre if (TREE_CODE (node) != FUNCTION_DECL && TREE_CODE (node) != VAR_DECL - && TREE_CODE (node) != TYPE_DECL) + && TREE_CODE (node) != TYPE_DECL + && TREE_CODE (node) != NAMESPACE_DECL) { *no_add_attrs = true; warning (OPT_Wattributes, "%qE attribute ignored", Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 154010) +++ gcc/cp/parser.c (working copy) @@ -12854,7 +12854,7 @@ cp_parser_namespace_definition (cp_parser* parser) push_namespace (identifier); } - has_visibility = handle_namespace_attrs (current_namespace, attribs); + has_visibility = handle_namespace_attrs (¤t_namespace, attribs); /* Parse the body of the namespace. */ cp_parser_namespace_body (parser); Index: gcc/cp/name-lookup.c =================================================================== --- gcc/cp/name-lookup.c (revision 154010) +++ gcc/cp/name-lookup.c (working copy) @@ -3192,7 +3192,7 @@ current_decl_namespace (void) NAMESPACE_DECL at all. Returns true if attribute visibility is seen. */ bool -handle_namespace_attrs (tree ns, tree attributes) +handle_namespace_attrs (tree *ns, tree attributes) { tree d; bool saw_vis = false; @@ -3214,7 +3214,7 @@ bool continue; } - if (!TREE_PUBLIC (ns)) + if (!TREE_PUBLIC (*ns)) warning (OPT_Wattributes, "%qD attribute is meaningless since members of the " "anonymous namespace get local symbols", name); @@ -3224,7 +3224,12 @@ bool } else #endif + if (is_attribute_p ("dllimport", name) || is_attribute_p ("dllexport", name)) { + cplus_decl_attributes (ns, attributes, 0); + } + else + { warning (OPT_Wattributes, "%qD attribute directive ignored", name); continue; Index: gcc/cp/name-lookup.h =================================================================== --- gcc/cp/name-lookup.h (revision 154010) +++ gcc/cp/name-lookup.h (working copy) @@ -304,7 +304,7 @@ extern void push_namespace (tree); extern void pop_namespace (void); extern void push_nested_namespace (tree); extern void pop_nested_namespace (tree); -extern bool handle_namespace_attrs (tree, tree); +extern bool handle_namespace_attrs (tree *, tree); extern void pushlevel_class (void); extern void poplevel_class (void); extern tree pushdecl_with_scope (tree, cxx_scope *, bool); Index: gcc/config/i386/winnt-stubs.c =================================================================== --- gcc/config/i386/winnt-stubs.c (revision 154010) +++ gcc/config/i386/winnt-stubs.c (working copy) @@ -46,6 +46,11 @@ i386_pe_type_dllexport_p (tree decl ATTRIBUTE_UNUS return false; } +bool +inherit_dllattribute_from_namespace (const char *attrname ATTRIBUTE_UNUSED, tree context ATTRIBUTE_UNUSED) +{ + return false; +} void i386_pe_adjust_class_at_definition (tree t ATTRIBUTE_UNUSED) Index: gcc/config/i386/winnt.c =================================================================== --- gcc/config/i386/winnt.c (revision 154010) +++ gcc/config/i386/winnt.c (working copy) @@ -112,7 +112,7 @@ i386_pe_determine_dllexport_p (tree decl) if (lookup_attribute ("dllexport", DECL_ATTRIBUTES (decl))) return true; - return false; + return inherit_dllattribute_from_namespace ("dllexport", /*DECL_CONTEXT*/ (decl)); } /* Return true if DECL should be a dllimport'd object. */ @@ -143,7 +143,7 @@ i386_pe_determine_dllimport_p (tree decl) error ("definition of static data member %q+D of " "dllimport'd class", decl); - return false; + return inherit_dllattribute_from_namespace ("dllimport", /*DECL_CONTEXT*/ (decl)); } /* Handle the -mno-fun-dllimport target switch. */ @@ -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/winnt-cxx.c =================================================================== --- gcc/config/i386/winnt-cxx.c (revision 154010) +++ gcc/config/i386/winnt-cxx.c (working copy) @@ -91,15 +91,115 @@ static inline void maybe_add_dllexport (tree decl) } } +static bool +inherit_dllattribute_from_namespace_1 (const char *attrname, tree context) +{ + if (context == NULL_TREE || context == global_namespace) + return false; + + /*fflush (0); fprintf (stderr, "inherit1: %s\n", attrname); fflush (0); + debug_tree (context);*/ + + if (CLASS_TYPE_P (context)) + { + return lookup_attribute (attrname, TYPE_ATTRIBUTES (context)) != NULL_TREE + || inherit_dllattribute_from_namespace_1 (attrname, CP_TYPE_CONTEXT (context)); + } + else if (TREE_CODE (context) == NAMESPACE_DECL) + { + return lookup_attribute (attrname, DECL_ATTRIBUTES (context)) != NULL_TREE + || (inherit_dllattribute_from_namespace_1 (attrname, CP_DECL_CONTEXT (context))); + } + else if (TREE_CODE (context) == FUNCTION_DECL) + { + /* Types declared in a function don't get exported just because + the function itself is, so we don't call lookup_attribute. */ + /* If the context is a nested function, don't escape. */ + return (TREE_CODE (CP_DECL_CONTEXT (context)) != FUNCTION_DECL + && inherit_dllattribute_from_namespace_1 (attrname, CP_DECL_CONTEXT (context))); + } + + fflush (0); fprintf (stderr, "unknown tree code\n"); fflush (0); + debug_tree (context); + + return false; +} + +/* The only valid inputs T to this function are VAR_DECL and FUNCTION_DECL + nodes passed in from i386_pe_determine_dll{im,ex}port_p, and class type + nodes passed in from i386_pe_adjust_class_at_definition. */ +bool +inherit_dllattribute_from_namespace (const char *attrname, tree t) +{ + tree tinfo = NULL_TREE; + tree context = NULL_TREE; + + /*fflush (0); fprintf (stderr, "inherit: %s\n", attrname); fflush (0); + debug_tree (t);*/ + + if (t == NULL_TREE) + return false; + + if (CLASS_TYPE_P (t)) + context = CP_TYPE_CONTEXT (t); + else if (TREE_CODE (t) == VAR_DECL || TREE_CODE (t) == FUNCTION_DECL) + context = CP_DECL_CONTEXT (t); + + if (inherit_dllattribute_from_namespace_1 (attrname, context)) + return true; + + /*fflush (0); fprintf (stderr, "inherit2: %s\n", attrname); fflush (0); + debug_tree (context);*/ + + /*if (context == NULL_TREE || context == global_namespace) + return false;*/ + + if ((TREE_CODE (t) == VAR_DECL || TREE_CODE (t) == FUNCTION_DECL) + && DECL_LANG_SPECIFIC (t) + && DECL_USE_TEMPLATE (t) + && DECL_TEMPLATE_INFO (t)) + { + tinfo = DECL_TEMPLATE_INFO (t); + } + else if (CLASS_TYPE_P (t) + && TYPE_LANG_SPECIFIC (t) + && CLASSTYPE_USE_TEMPLATE (t) + && CLASSTYPE_TEMPLATE_INFO (t)) + { + tinfo = CLASSTYPE_TEMPLATE_INFO (t); + } + + if (tinfo) + { + tree ti_template = TI_TEMPLATE (tinfo); + /*fflush (0); fprintf (stderr, "inherit3: %s\n", attrname); fflush (0); + debug_tree (ti_template);*/ + gcc_assert (TREE_CODE (ti_template) == TEMPLATE_DECL); + return inherit_dllattribute_from_namespace_1 (attrname, CP_DECL_CONTEXT (ti_template)); + } + + return false; +} + void i386_pe_adjust_class_at_definition (tree t) { tree member; + bool is_import, is_export; gcc_assert (CLASS_TYPE_P (t)); + is_export = lookup_attribute ("dllexport", TYPE_ATTRIBUTES (t)) != NULL_TREE; + if (!is_export) + is_export = inherit_dllattribute_from_namespace ("dllexport", /*CP_TYPE_CONTEXT*/ (t)); + if (!is_export) + { + is_import = lookup_attribute ("dllimport", TYPE_ATTRIBUTES (t)) != NULL_TREE; + if (!is_export && !is_import) + is_import = inherit_dllattribute_from_namespace ("dllimport", /*CP_TYPE_CONTEXT*/ (t)); + } - if (lookup_attribute ("dllexport", TYPE_ATTRIBUTES (t)) != NULL_TREE) + if (is_export) { /* Check static VAR_DECL's. */ for (member = TYPE_FIELDS (t); member; member = TREE_CHAIN (member)) @@ -124,7 +224,7 @@ i386_pe_adjust_class_at_definition (tree t) maybe_add_dllexport (member); } - else if (lookup_attribute ("dllimport", TYPE_ATTRIBUTES (t)) != NULL_TREE) + else if (is_import) { /* We don't actually add the attribute to the decl, just set the flag that signals that the address of this symbol is not a compile-time Index: gcc/config/i386/i386-protos.h =================================================================== --- gcc/config/i386/i386-protos.h (revision 154010) +++ gcc/config/i386/i386-protos.h (working copy) @@ -240,6 +240,7 @@ extern void i386_pe_file_end (void); extern tree i386_pe_mangle_decl_assembler_name (tree, tree); /* In winnt-cxx.c and winnt-stubs.c */ +extern bool inherit_dllattribute_from_namespace (const char *, tree); extern void i386_pe_adjust_class_at_definition (tree); extern bool i386_pe_type_dllimport_p (tree); extern bool i386_pe_type_dllexport_p (tree); --------------070505020000090202030603--