With both 3.4.0 (20040327) and trunk (20040331), I am getting segfaults or unexpected results with virtual base classes on i386-pc-mingw32 Here is an illustration: // vbase.cc #include <stdio.h> class Base { public: // int base_var; virtual char* vf() const = 0; virtual ~Base() {} }; class D1 : public Base { public: char* vf() const { return "D1"; } }; class D2 : virtual public Base { public: char* vf() const { return "D2"; } }; void test1() { Base* b1 = new D1; printf("%s\n", b1->vf()); } void test2() { Base* b2 = new D2; printf("%s\n", b2->vf()); } int main() { test1(); test2(); return 0; } //end vbase.cc With trunk: When compiled without optimisation g++ -ovbase0 -Wall -O0 vbase.cc I get successful build, with no warnings. But executing vbase0 causes segfault. Ditto with -O1 When compiled with -O2 optimisation g++ -ovbase2 -Wall -O2 vbase.cc executing vbase2 outputs: D1 2D2 When I add -g: g++ -ovbase0g -g -Wall -O0 vbase.cc Executing vbase0g doesn't segfault. But it doesn't produce _any_ output either. Ditto with -O1 -g. This is regression from 3.3.3 which output D1 D2 at all levels of optimisation 2.95.3 also produced the correct output. If I add a variable (the commented out int base_var) to the base class, the executable built with 3.4.0 and trunk executes without problem and produces the correct output. If I comment out the "virtual" keyword in D2's definition, I also get correct output with 3.4.0 and trunk. Danny
Hello I suspect that this bug is due to the way that the PE-COFF emulation of GNU ld handles linkonce sections. Specifically, ld does not reliably handle linkonce sections contining more than one global symbol. For example, this assembler code also produces a faulting app: .file "bar2.c" .def ___main; .scl 2; .type 32; .endef .text .align 2 .globl _main .def _main; .scl 2; .type 32; .endef _main: pushl %ebp movl %esp, %ebp subl $8, %esp andl $-16, %esp movl $16, %eax call __alloca call ___main call __Z3onev call __Z8call_onev leave .p2align 4,,4 ret .section .text$_Z8call_onev,"x" .linkonce discard .align 2 .globl __Z8call_onev .def __Z8call_onev; .scl 2; .type 32; .endef __Z8call_onev: subl $12, %esp call __Z3onev addl $12, %esp ret # .section .text$_Z3onev,"x" # .linkonce discard .align 2 .globl __Z3onev .def __Z3onev; .scl 2; .type 32; .endef __Z3onev: movl $1, %eax ret but is okay if I put the the the two functions in separate .linkonce sections, Referring to the testcase in my first comments for this PR, if the virtual function in the derived class is not inlined, and so does not use linkonce semantics, then the app built from the modified testcase executes correctly. Likewise, if I comment out this code in cp/method,c (use_thunk) *** method.c.orig Mon Apr 05 21:49:30 2004 --- method.c Mon Apr 05 00:41:59 2004 *************** use_thunk (tree thunk_fndecl, bool emit_ *** 409,416 **** --- 409,418 ---- if (DECL_SECTION_NAME (function) != NULL && DECL_ONE_ONLY (function)) { resolve_unique_section (thunk_fndecl, 0, flag_function_sections); + #if !(defined (__MINGW32__ ) || defined (__CYGWIN__)) /* Output the thunk into the same section as function. */ DECL_SECTION_NAME (thunk_fndecl) = DECL_SECTION_NAME (function); + #endif } } #endif the app is okay. But, as indicated by Jakub when the local aliases for virtual thunks code was introduced http://gcc.gnu.org/ml/gcc-patches/2003-06/msg02603.html "If a method is emitted in a linkonce section, it will emit the thunk into the same linkonce section as well, since local symbols in linkonce sections cannot be safely accessed from other sections." this is not really a safe thing to do. Should _all_ the make_alias_for_thunk code in method.c be guarded by a target macro (e.g. TARGET_USE_LOCAL_ALIAS_FOR_VTHUNK), which defaults to 1 #ifdef ASM_OUTPUT_DEF, but can be overriden by targets which define ASM_OUTPUT_DEF, but may currently have problems with multiple globals in linkonce sections? Danny
Patch submitted: http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00432.html
Confirmed I know this is too close but still I think this should be fixed for 3.4.0.
The patch is OK for the mainline. On the 3.4.0 branch, it is *not* OK. Instead, please submit an uglier, but safer, patch that tests for __CYGWIN__ and __MINGW32__ in the same place where the mainline patch checks the new target macro. In other words: #if defined (ASM_OUTPUT_DEF) && !defined(__CYGWIN__) && !... alias version #else non-alias version #endif Please attach that patch to the PR after testing. I will then approve it for 3.4.0.
Created attachment 6063 [details] patch against 3.4.0 prerelease Attached is patch against Reading specs from d:/mingw/bin/../lib/gcc/mingw32/3.4.0/specs Configured with: ../gcc/configure --with-gcc --with-gnu-ld --with-gnu-as --host=mingw32 --target=mingw32 --prefix=/mingw --enable-threads --disable-nls --enable-languages=c,c++,objc --disable-win32-registry --disable-shared Thread model: win32 gcc version 3.4.0 20040406 (prerelease) It fixes 54 g++ testsuite regressions, no new ones introduced. cp/Changelog 2004-04-08 Danny Smith <dannysmith@users.sourceforge.net> * method.c (make_alias_for_thunk): Just return function decl for one_only functions if __CYGWIN__ or __MINGW32__ (use_thunk): Don't put function and thunk in same one_only section if __CYGWIN__ or __MINGW32__.
This patch is OK for 3.4.0.
Subject: Bug 14808 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: dannysmith@gcc.gnu.org 2004-04-08 22:16:01 Modified files: gcc/cp : ChangeLog method.c Log message: PR c++/14808 * method.c (make_alias_for_thunk): Just return function decl for one_only functions if __CYGWIN__ or __MINGW32__ (use_thunk): Don't put function and thunk in same one_only section if __CYGWIN__ or __MINGW32__. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.95&r2=1.3892.2.96 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.275.4.2&r2=1.275.4.3
Subject: Bug 14808 CVSROOT: /cvs/gcc Module name: gcc Changes by: dannysmith@gcc.gnu.org 2004-04-08 22:31:23 Modified files: gcc : ChangeLog defaults.h gcc/config/i386: cygming.h gcc/cp : ChangeLog method.c gcc/doc : tm.texi Log message: PR c++/14808 * defaults.h (TARGET_USE_LOCAL_THUNK_ALIAS_P): New macro. Default to 1 if ASM_OUTPUT_DEF is defined. * doc/tm.texi (TARGET_USE_LOCAL_THUNK_ALIAS_P): Document. * config/i386/cygming.h (TARGET_USE_LOCAL_THUNK_ALIAS_P): Define. Set to non-zero iff not a one_only decl. * cp/method.c (use_thunk): Test TARGET_USE_LOCAL_THUNK_ALIAS_P rather than ASM_OUTPUT_DEF. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3367&r2=2.3368 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/defaults.h.diff?cvsroot=gcc&r1=1.132&r2=1.133 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/cygming.h.diff?cvsroot=gcc&r1=1.13&r2=1.14 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4030&r2=1.4031 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&r1=1.279&r2=1.280 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/doc/tm.texi.diff?cvsroot=gcc&r1=1.317&r2=1.318
Fixed on 3.4.0 and trunk Danny
Turns out this patch may have caused cygwin->linux crossbuilds to fail, since in that case __CYGWIN__ is defined even though the target isn't cygwin. The symptom is strstream.s:8390: Error: junk `(%ecx)' after expression when building libstdc++. See http://gcc.gnu.org/ml/gcc/2004-06/msg00429.html for a patch that lets my builds finish.
Subject: Re: [3.4/3.5 Regression] [win32] Undefined results with virtual base classes dank at kegel dot com wrote: > ------- Additional Comments From dank at kegel dot com 2004-06-07 12:15 ------- > Turns out this patch may have caused cygwin->linux crossbuilds to > fail, since in that case __CYGWIN__ is defined even though the > target isn't cygwin. The symptom is > strstream.s:8390: Error: junk `(%ecx)' after expression > when building libstdc++. > See http://gcc.gnu.org/ml/gcc/2004-06/msg00429.html for a patch > that lets my builds finish. This patch is fine for 3.4.1; please check it in.
Subject: Bug 14808 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: wilson@gcc.gnu.org 2004-06-08 06:30:33 Modified files: gcc : ChangeLog gcc/config/i386: cygwin.h cygming.h gcc/cp : ChangeLog method.c Log message: Patch from Dan Kegel. PR c++/14808 * config/i386/cygwin.h (TARGET_IS_PE_COFF): New. * config/i386/cygming.h (TARGET_IS_PE_COFF): New. * method.c (make_alias_for_thunk, use_thunk): Use TARGET_IS_PE_COFF instead of __CYWGIN__ and __MINGW32__. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.477&r2=2.2326.2.478 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/cygwin.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.85&r2=1.85.10.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/cygming.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.11.4.1&r2=1.11.4.2 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.117&r2=1.3892.2.118 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.275.4.3&r2=1.275.4.4
Subject: Re: [3.4/3.5 Regression] [win32] Undefined results with virtual base classes mark at codesourcery dot com wrote: > This patch is fine for 3.4.1; please check it in. Dan Kegel isn't in the MAINTAINERS file. I checked in the patch. I had to apply it by hand. (Dan: please use MIME attachments for patches.) Because the patch is small, and is essentially what I suggested, I don't believe any copyright assignment is needed. In addition to fixing cygwin-x-linux cross compilers, this will also fix linux-x-cygwin cross compilers. We have had a number of bugs reported against the latter, but I don't think anyone ever tried to track down the problem.
Subject: Bug 14808 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: wilson@gcc.gnu.org 2004-06-16 22:59:42 Modified files: gcc : ChangeLog gcc/config/i386: cygwin.h Log message: PR c++/14808 * config/i386/cygwin.h (TARGET_IS_PE_COFF): Delete duplicate macro. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.510&r2=2.2326.2.511 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/cygwin.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.85.10.1&r2=1.85.10.2