Workaround for virtual function/dllimport bug, also patch guidance request

[Mumit Khan]

Jason Merrill <jason@cygnus.com> writes:
> >>>>> Mumit Khan <khan@nanotech.wisc.edu> writes:
> 
>  >     For subclasses of dllimported functions, vtables reference the
>  >     *thunk* functions, not the synthetically created indirect references
>  >     (by backend in i386_pe_mark_dllimport); this is needed to allow 
>  >     taking the address of the virtual function. This is one thing that 
>  >     needs to be changed in current GCC implementation (eg., by 
>  >     nullifying the dllimport attribute of virtual functions in the 
>  >     backend, or dealing with specifically in the front-end).
> 
> Shouldn't the backend just treat code that wants the address of a function
> differently?

Yes, it should, and it does. I believe I have the changes needed to get the 
whole mess to work, w/out any change to current C++ front-end. The trick
here is to make sure virtual functions (* see below) and vtables are not
imported; if not, the vtable generation will fail. In fact, since vtables
are always generated (MULTIPLE_SYMBOL_SPACES in import_export_class), we
don't need to import vtables by reference from a DLL, and it's always works.

(*) unfortunately, I can't find any way in the target-dependent file to
figure out if a function is really virtual or not just by checking
DECL_VIRTUAL_P or DECL_VINDEX when marking dllimports. It works for:
  
  struct IMPORT base {
    virtual void foo ();
  }
  struct IMPORT derived1 {
    virtual void foo ();
  }

but not for,
  
  struct IMPORT base {
    virtual void foo ();
  }
  struct IMPORT derived1 {
    void foo ();
  }

So, I have to disable importing of all methods. This has no effect on the
user code other than a tiny penalty in calling the thunk instead.

Jason, ideas? here's the patch to i386/winnt.c, which fixes all the bug
reports of this type I've received to date. Unless you have an objection
to this, I'll submit it to gcc-patches after a bit of cleanup. The term
"thunk" used here is the thunk in the import library, and not as in
vtable thunks.

Fri Dec 10 19:29:48 1999  Mumit Khan  <khan@xraylith.wisc.edu>

	* i386/config/winnt.c (i386_pe_dllimport_p): Don't import inline
	functions, and C++ methods and vtables.
	* (i386_pe_mark_dllimport): Handle illegal dllimports.

Index: winnt.c
===================================================================
RCS file: /homes/khan/src/CVSROOT/gcc-2.95.2/gcc/config/i386/winnt.c,v
retrieving revision 1.4
diff -u -3 -p -r1.4 winnt.c
--- winnt.c	1999/11/05 08:21:44	1.4
+++ winnt.c	1999/12/11 01:39:02
@@ -214,6 +214,34 @@ i386_pe_dllimport_p (decl)
   if (TREE_CODE (decl) != VAR_DECL
       && TREE_CODE (decl) != FUNCTION_DECL)
     return 0;
+
+  /* We ignore the attribute for inline functions. */
+  if (TREE_CODE (decl) == FUNCTION_DECL 
+      && DECL_INITIAL (decl) 
+      && DECL_INLINE (decl) 
+      && TREE_CODE (TREE_TYPE (decl)) != METHOD_TYPE)
+    {
+      return 0;
+    }
+  
+  /* Also turn off importing C++ virtual methods to able to create 
+     vtables using thunks. 
+     
+     FIXME: However, since there is no way to tell if the type is 
+     really virtual at this point (the declaration of the method in 
+     a derived class may not have the virtual keyword), we just turn 
+     off all methods at the expense of slight performance penalty.  */
+  if (TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
+    {
+      return 0;
+    }
+
+  /* Likewise for vtables. */
+  if (TREE_CODE (decl) == VAR_DECL && DECL_VIRTUAL_P (decl))
+    {
+      return 0;
+    }
+
   imp = lookup_attribute ("dllimport", DECL_MACHINE_ATTRIBUTES (decl));
   if (imp)
     return 1;
@@ -342,6 +370,7 @@ i386_pe_mark_dllimport (decl)
       error_with_decl (decl, "initialized variable `%s' is marked dllimport");
       return;
     }
+
   /* Nor can they be static.  */
   if (TREE_CODE (decl) == VAR_DECL
       /* ??? Is this test for vtables needed?  */
@@ -352,6 +381,14 @@ i386_pe_mark_dllimport (decl)
       return;
     }
 
+  /* Nor can you define an imported function. */
+  if (TREE_CODE (decl) == FUNCTION_DECL 
+      && DECL_INITIAL (decl) && ! DECL_INLINE (decl))
+    {
+      error_with_decl (decl, "function `%s' definition is marked dllimport");
+      return;
+    }
+  
   /* `extern' needn't be specified with dllimport.
      Specify `extern' now and hope for the best.  Sigh.  */
   if (TREE_CODE (decl) == VAR_DECL
@@ -361,7 +398,7 @@ i386_pe_mark_dllimport (decl)
       DECL_EXTERNAL (decl) = 1;
       TREE_PUBLIC (decl) = 1;
     }
-
+  
   newname = alloca (strlen (oldname) + 11);
   sprintf (newname, "@i._imp__%s", oldname);
 
Regards,
Mumit