Bug 40068 - GCC fails to apply dllexport attribute to typeinfo.
Summary: GCC fails to apply dllexport attribute to typeinfo.
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Danny Smith
URL:
Keywords:
: 50044 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-05-08 11:52 UTC by Dave Korn
Modified: 2013-09-13 07:54 UTC (History)
3 users (show)

See Also:
Host: i686-pc-cygwin
Target: i686-pc-cygwin
Build: i686-pc-cygwin
Known to work:
Known to fail:
Last reconfirmed: 2009-05-10 05:01:24


Attachments
Simple testcase (144 bytes, application/octet-stream)
2009-05-08 11:53 UTC, Dave Korn
Details
inherit dllexport from class to typeinfo (763 bytes, patch)
2009-05-10 01:11 UTC, Dave Korn
Details | Diff
missing dllexport on typeinfo output (742 bytes, text/plain)
2013-09-13 07:54 UTC, Matt Clarkson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Korn 2009-05-08 11:52:42 UTC
Precisely as subject.  All other class members and related data (e.g. vtables, synthetic ctors) will be tagged dllexport if the class itself is dllexport, and -export comments will be added listing their symbols in the .drectve section.  Typeinfo data alone will not receive the dllexport attribute, because it is not a class member.  (Theoretically, I think it counts as a static class data member of type std::type_info, but internally) It is generated as a synthetic POD data type, and hence does not appear to the i386 backend as belonging to the class it represents.

The description in bug 17300 suggests that there could be a problem further down the line with dllimporting such typeinfos, but right now they simply can't be referenced outside the enclosing DLL at all, i.e. clients of a DLL can't dynamically cast class pointers to the classes it implements, only routines within the DLL which have local linkage to the typeinfo.  This is a separate problem, which I think can be resolved further down the toolchain using pseudo-relocs; I don't think it's a reason not to make export work.
Comment 1 Dave Korn 2009-05-08 11:53:44 UTC
Created attachment 17826 [details]
Simple testcase

It doesn't get much more trivial than this.
Comment 2 Dave Korn 2009-05-08 11:55:51 UTC
To reproduce the bug, compile the attached testcase

g++-4 -fpreprocessed -S vti.ii

and view the very end of the .s file emitted:

	.section .drectve
	.ascii " -export:_ZTV12XMLException,data"
	.ascii " -export:_ZN12XMLExceptionD2Ev"
	.ascii " -export:_ZN12XMLExceptionD1Ev"
	.ascii " -export:_ZN12XMLExceptionD0Ev"
	.ascii " -export:_ZN12XMLExceptionC1Ev"
	.ascii " -export:_ZN12XMLExceptionC2Ev"

Everything gets exported except the typeinfo.
Comment 3 Dave Korn 2009-05-10 01:11:27 UTC
Created attachment 17841 [details]
inherit dllexport from class to typeinfo

Now testing a solution based on the approach of adding the dllexport attribute to the class' typeinfo object when the class is passed to i386_pe_adjust_class_at_definition.
Comment 4 Danny Smith 2009-05-10 05:01:23 UTC
(In reply to comment #3)
> Created an attachment (id=17841) [edit]
> inherit dllexport from class to typeinfo
> 
> Now testing a solution based on the approach of adding the dllexport attribute
> to the class' typeinfo object when the class is passed to
> i386_pe_adjust_class_at_definition.
> 
Hello Dave,
Rather than use DLL linkage (and so force client to resort to auto-import magic)
why not just always emit the RTTI with one-only comdat linkage.

	* decl2.c (import_export_decl): Always emit RTTI with comdat linkage
	rather than import if MULTIPLE_SYMBOL_SPACES.

Index: decl2.c
===================================================================
--- decl2.c	(revision 147015)
+++ decl2.c	(working copy)
@@ -2351,15 +2351,21 @@
 	{
 	  class_type = type;
 	  import_export_class (type);
-	  if (CLASSTYPE_INTERFACE_KNOWN (type)
-	      && TYPE_POLYMORPHIC_P (type)
-	      && CLASSTYPE_INTERFACE_ONLY (type)
-	      /* If -fno-rtti was specified, then we cannot be sure
-		 that RTTI information will be emitted with the
-		 virtual table of the class, so we must emit it
-		 wherever it is used.  */
-	      && flag_rtti)
-	    import_p = true;
+          /* Do not import typeinfo if the class might be in a DLL.
+	     Dllimports do not have a constant address at compile time,
+	     causing problems for static initialization of tables with RTTI
+	     fields.  Set to comdat instead.   */
+	  if (MULTIPLE_SYMBOL_SPACES)
+  	    comdat_p = true;
+	  else if (CLASSTYPE_INTERFACE_KNOWN (type)
+		   && TYPE_POLYMORPHIC_P (type)
+		   && CLASSTYPE_INTERFACE_ONLY (type)
+		   /* If -fno-rtti was specified, then we cannot be sure
+		      that RTTI information will be emitted with the
+ 		      virtual table of the class, so we must emit it
+ 		      wherever it is used.  */
+	           && flag_rtti)
+  	     import_p = true;
 	  else
 	    {
 	      if (CLASSTYPE_INTERFACE_KNOWN (type)
Comment 5 Dave Korn 2009-05-10 11:17:51 UTC
(In reply to comment #4)
> Hello Dave,

  Hi Danny!

> Rather than use DLL linkage (and so force client to resort to auto-import
> magic)
> why not just always emit the RTTI with one-only comdat linkage.

  I have your patch in the cygwin distro compiler where it works fine, but I am concerned about what unforeseeable problems could arise by violating ODR in this way.  I don't have any concrete evidence of any problem yet, it is just a worry.

  Also, I don't think this is necessarily an either-or situation; we could add my patch and have the typeinfo exported from the DLL, and also add yours so that clients could link a comdat copy (which would override the import stub) until we have a better solution for importing from the DLL.  Or I could follow up with another patch that propagates dllimport attributes from class to typeinfo.

There is of course this:

/* We leave typeinfo tables alone.  We can't mark TI objects as
     dllimport, since the address of a secondary VTT may be needed
     for static initialization of a primary VTT.  VTT's  of
     dllimport'd classes should always be link-once COMDAT.  */ 

	  /* Do not import tinfo nodes if the class has dllimport attribute.
	     Dllimports do not have a constant address at compile time, so
	     static initialization of tables with RTTI fields is a problem.
	     Set to comdat instead.   */

... but I do not see why this should be a problem in these days of auto-import and pseudo-relocs; do you know more about what the actual problem is (or was) here?  Is this not basically the same situation as something like 

----dll_A.c----

int foo;
int __attribute__ ((dllexport)) * bar = &foo;

----dll_B.c----

extern int __attribute__ ((dllexport)) * bar;
int __attribute__ ((dllexport)) * baz = &bar;

---------------

the example above?

  If it's possible to solve either in the compiler or further down the toolchain, I would very much like to do so.

  Also, how come emitting the typeinfo .linkonce as we currently do isn't the same as COMDAT for these purposes?

    cheers,
      DaveK
Comment 6 Danny Smith 2009-05-13 08:12:30 UTC
(In reply to comment #5)

>   Also, I don't think this is necessarily an either-or situation; we could add
> my patch and have the typeinfo exported from the DLL, and also add yours so
> that clients could link a comdat copy (which would override the import stub)
> until we have a better solution for importing from the DLL.

Yes.  I was just thinking out loud.  I have no objection to your patch.  Perhaps MULTIPLE_SYMBOL_SPACES should be defined in terms of an -mmultiple-symbol-spaces target option , since if we depend on auto-import than we should treat dll imports thae same as static lib imports.

> Also, how come emitting the typeinfo .linkonce as we currently do isn't the
> same as COMDAT for these purposes?
> 
I'm not sure I understand your question.  In earlier versions of G++,  both vtables and typeinfo were  always emitted with linkonce semantics, because of the MULTIPLE_SYMBOL_SPACES define.  

Regards
Danny


Comment 7 Dave Korn 2009-06-25 15:37:05 UTC
  Hmm, I'm getting somewhere with this.

  By compiling the g++ testsuite "ptrflags.C" case with --save-temps, manually hacking all the superfluous typeinfo stuff out, and re-assembling and linking it, I can turn a FAIL into a PASS, with all the typeinfo and related stuff being imported from the DLL.

  So I think typeinfo can be made to work, and work with both __GXX_MERGED_TYPEINFO_NAMES and __GXX_TYPEINFO_EQUALITY_INLINE.

  All I need to do is stop the compiler from emitting comdat definitions of items with vague linkage when they are dllimported, or perhaps make the linker prefer an import over a comdat section when the same symbol could be resolved by either.  Or perhaps both.  I think the linker fix is what you were suggesting when you said

> since if we depend on auto-import than
> we should treat dll imports thae same as static lib imports

  More later.
Comment 8 Kai Tietz 2012-02-02 21:00:41 UTC
*** Bug 50044 has been marked as a duplicate of this bug. ***
Comment 9 Kai Tietz 2012-02-07 10:46:06 UTC
Author: ktietz
Date: Tue Feb  7 10:45:59 2012
New Revision: 183962

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183962
Log:
2012-02-07  Kai Tietz  <ktietz@redhat.com>
            Dave Korn  <dave.korn.cygwin@gmail.com>

        PR target/40068
        * config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition):
        Take care that typinfo gets dllexport-attribute.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/winnt-cxx.c
Comment 10 Kai Tietz 2012-02-07 10:49:20 UTC
Author: ktietz
Date: Tue Feb  7 10:49:14 2012
New Revision: 183963

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183963
Log:
2012-02-07  Kai Tietz  <ktietz@redhat.com>
            Dave Korn  <dave.korn.cygwin@gmail.com>

        PR target/40068
        * config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition):
        Take care that typinfo gets dllexport-attribute.


Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/i386/winnt-cxx.c
Comment 11 Kai Tietz 2012-02-07 11:48:41 UTC
Author: ktietz
Date: Tue Feb  7 11:48:34 2012
New Revision: 183965

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183965
Log:
2012-02-07  Kai Tietz  <ktietz@redhat.com>
            Dave Korn  <dave.korn.cygwin@gmail.com>

        PR target/40068
        * config/i386/winnt-cxx.c (i386_pe_adjust_class_at_definition):
        Take care that typinfo gets dllexport-attribute.


Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/i386/winnt-cxx.c
Comment 12 Matt Clarkson 2013-09-13 07:52:04 UTC
Did that patch resolve this bug?  I'm using MinGW-builds-4.8.1-x64-posix-seh and building a project and ended up with the following error:

value.hpp:33:31: error: external linkage required for symbol '_ZTIN3udp8keystone6binary9arguments5ValueIZNS1_4Main17GetParsedArgumentERKSsE5DummyEE' because of 'dllexport' attribute class UDP_KEYSTONE_DLL_PUBLIC Value final : public ValueBase {

Which translates to missing 'dllexport' attribute on 'typeinfo for udp::keystone::binary::arguments::Value'

But both my Value class and ValueBase classes are marked with 'UDP_KEYSTONE_DLL_PUBLIC' that expands to '__attribute__((dllexport))'

If this is a similar bug and not my error (which I presume it is) I will resolve a test delta.
Comment 13 Matt Clarkson 2013-09-13 07:54:12 UTC
Created attachment 30814 [details]
missing dllexport on typeinfo output

Added an attachment for the full compiler output.