Bug 36654 - [4.2 Regression] Inlined con/de-structor breaks virtual inheritance dllimport classes
Summary: [4.2 Regression] Inlined con/de-structor breaks virtual inheritance dllimport...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.3.1
: P2 major
Target Milestone: 4.3.3
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2008-06-27 16:55 UTC by John E. / TDM
Modified: 2009-03-31 15:42 UTC (History)
4 users (show)

See Also:
Host: mingw32
Target: mingw32
Build: mingw32
Known to work: 3.4.5 4.2.1 4.3.3
Known to fail: 4.2.4 4.3.0 4.3.1 4.3.2 4.2.5
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John E. / TDM 2008-06-27 16:55:03 UTC
In mingw32 builds of the GCC 4.3 branch, the following code causes an ICE:

class blah {};

class __attribute__((dllimport)) vchild : virtual public blah
{
	vchild() {}
};

The dllimport attribute, the virtual inheritance, and the inline constructor are key to causing the error. A Windows prompt session using the MinGW GCC 4.3.0 alpha follows:

> g++ -v
Using built-in specs.
Target: mingw32
Configured with: ../gcc-4.3.0/configure --enable-languages=c,ada,c++,fortran,java,objc,obj-c++ --disable-sjlj-exceptions --enable-shared --enable-libgcj --enable-libgomp --with-dwarf2 --disable-win32-registry --enable-libstdcxx-debug --enable-concept-checks --enable-version-specific-runtime-libs --build=mingw32 --with-bugurl=http://www.mingw.org/bugs.shtml --prefix=/mingw --with-gmp=/mingw/src/gcc/gmp-mpfr-root --with-mpfr=/mingw/src/gcc/gmp-mpfr-root --with-libiconv-prefix=/mingw/src/gcc/libiconv-root
Thread model: win32
gcc version 4.3.0 20080305 (alpha-testing) mingw-20080502 (GCC)

> g++ -c deltatest.ii
deltatest.ii:8: internal compiler error: in maybe_emit_vtables, at cp/decl2.c:1678
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://www.mingw.org/bugs.shtml> for instructions.
Comment 1 John E. / TDM 2008-06-27 17:09:07 UTC
I have also discovered that this bug is *not* present in the MinGW 4.2.1 TP (or in MinGW 3.4.5), but is present in unofficial builds of GCC 4.3.1 and 4.2.4 as well as 4.3.0.
Comment 2 Danny Smith 2008-06-28 07:53:06 UTC
This is duplicate of 35921 and is fixed on 4.3.2 and trunk. 

*** This bug has been marked as a duplicate of 35921 ***
Comment 3 John E. / TDM 2008-08-12 02:13:50 UTC
As of r138967 pulled down today from the 4.3 branch, this bug is in fact still present. Shell session follows.

> g++ -v
Using built-in specs.
Target: mingw32
Configured with: ../gcc-4.3-svn/configure --prefix=/mingw --build=mingw32 --enable-languages=c,c++ --with-bugurl=http://www.tdragon.net/recentgcc/bugs.php --disable-nls --disable-win32-registry --disable-werror --enable-threads --disable-symvers --enable-cxx-flags='-fno-function-sections -fno-data-sections' --enable-fully-dynamic-string --enable-version-specific-runtime-libs --enable-sjlj-exceptions --with-pkgversion='GCC TDM-1 for MinGW' --disable-bootstrap
Thread model: win32
gcc version 4.3.2 20080811 (prerelease) (GCC TDM-1 for MinGW)

> g++ -c deltatest.ii
deltatest.ii:8: internal compiler error: in maybe_emit_vtables, at cp/decl2.c:1745
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://www.tdragon.net/recentgcc/bugs.php> for instructions.
Comment 4 Joseph S. Myers 2008-08-27 22:04:55 UTC
4.3.2 is released, changing milestones to 4.3.3.
Comment 5 John E. / TDM 2008-09-20 17:57:50 UTC
Any news or thoughts on this bug? (*Ping*)
Comment 6 John E. / TDM 2008-10-09 22:57:00 UTC
Ping! Any ideas on this bug?
Comment 7 John E. / TDM 2008-11-02 00:54:49 UTC
C'mon, folks! Any chance of getting this fixed?
Comment 8 Danny Smith 2009-01-02 04:19:45 UTC
Hello John,
This patch:

Index: gcc/config/i386/winnt-cxx.c
===================================================================
--- gcc/config/i386/winnt-cxx.c	(revision 142383)
+++ gcc/config/i386/winnt-cxx.c	(working copy)
@@ -65,7 +65,7 @@
       ignore the class attribute.  */
   else if (TREE_CODE (decl) == VAR_DECL
 	   && TREE_STATIC (decl) && TREE_PUBLIC (decl)
-	   && !DECL_EXTERNAL (decl))
+	   && DECL_NOT_REALLY_EXTERN (decl))
     {
       if (!DECL_VIRTUAL_P (decl))
 	  error ("definition of static data member %q+D of "

fixes your testcase and causes no regressions in g++ testsuite.  I have only tested on narrow range of dll builds, to check that the patch does not break anything.  I think it may cause spurious errors with import of static data members, but that can be fixed.  Could you please test with your projects.

Thanks

Danny
Comment 9 John E. / TDM 2009-01-02 14:23:09 UTC
That patch seems to work fine, and I haven't seen any unwarranted errors relating to static data members so far. Thank you very much for tracking this down!
Comment 10 dannysmith@gcc.gnu.org 2009-01-07 07:33:11 UTC
Subject: Bug 36654

Author: dannysmith
Date: Wed Jan  7 07:32:58 2009
New Revision: 143150

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143150
Log:
	PR target/36654
	* config/i386/winnt-cxx.c (i386_pe_type_dllimport_p): Check
	DECL_NOT_REALLY_EXTERN rather than !DECL_EXTERNAL

Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/i386/winnt-cxx.c

Comment 11 Danny Smith 2009-01-07 07:47:11 UTC
Fixedd on 4.3 branch
Comment 12 Dave Korn 2009-03-25 08:03:32 UTC
Hi all.

This patch caused g++.dg/ext/dllimport7.C to regress (in one subtest) between 4.3.2 and 4.3.3 on Cygwin, although it could be that the testcase is out of date.

// { dg-do compile { target i?86-*-cygwin* i?86-*-mingw*} }

//  Report errors on definition of dllimport'd static data member . 


struct Baz
{
  Baz(int a_ =0) : a(a_) {} 
  int a;
};

class  __declspec(dllimport) Bar
{
  public:
    enum {one = 1};
    static const int two = 2;
    static const int three;
    static const Baz null_baz;
};

const int Bar::three = 3;       //  { dg-warning "redeclared without dllimport" }
//  { dg-error "definition of static data" "C++ specific error" { target i?86-*-cygwin* i?86-*-mingw* } 21 }
				
const Baz Bar::null_baz;	//  { dg-warning "redeclared without dllimport" }
//  { dg-error "definition of static data" "C++ specific error" { target i?86-*-cygwin* i?86-*-mingw* }  24 }



int foo()
{
  Bar foobar;
  const int* baz = &Bar::two; 
  int a = foobar.two;
  int b = foobar.three;
  int c = foobar.null_baz.a;
  return (a + b + c + *baz);
}



Both the dg-error clauses now fail; previously (4.3.2), only the second one failed.  Reverting the patch causes the first dg-error (line 21) to pass again by restoring the error message

/gnu/gcc/release/gcc4-4.3.3-1/src/gcc-4.3.3/gcc/testsuite/g++.dg/ext/dllimport7.
C:21: error: definition of static data member 'Bar::three' of dllimport'd class

I'm not sure why that should be a problem in the first place, so I don't know if the underlying issue is now fixed and not an error any more.  Anybody?
Comment 13 Danny Smith 2009-03-28 07:24:28 UTC
(In reply to comment #12)
> Both the dg-error clauses now fail; previously (4.3.2), only the second one
> failed.  Reverting the patch causes the first dg-error (line 21) to pass again
> by restoring the error message
> 
> /gnu/gcc/release/gcc4-4.3.3-1/src/gcc-4.3.3/gcc/testsuite/g++.dg/ext/dllimport7.
> C:21: error: definition of static data member 'Bar::three' of dllimport'd class
> 
> I'm not sure why that should be a problem in the first place, so I don't know
> if the underlying issue is now fixed and not an error any more.  Anybody?
> 

IMO, the hard error should occur.
The native MS compiler emits a similar error message.
Allowing a static data member to be defined in more than one place is a bad thing.  It most be defined in the dll if the class is defined there and, in the situation the testcase is testing will be defined again in the exe or dll that uses the dll.   

Comment 14 John E. / TDM 2009-03-29 15:30:27 UTC
I have just filed PR39578, the cause of which may be related to this PR or its fix.
Comment 15 Joseph S. Myers 2009-03-31 15:42:08 UTC
Closing 4.2 branch, fixed for 4.3.3 and 4.4.  If there are problems with
the fix, a separate issue has been / should be filed for those.