When the sample code snippet (below) is compiled, errors (below) occur. This issue breaks the compilation of wxWidgets' Media Library. The same problem occurs with the latest snapshot (20060331, used in the example below) of GCC 4.1.1, on both CygWin and MinGW. The sample code (virt-test.ii): # 1 "virt-test.cpp" # 1 "<built-in>" # 1 "<command line>" # 1 "virt-test.cpp" struct top { virtual ~top() {} virtual int __attribute__((__stdcall__)) fun1() = 0; virtual char __attribute__((__stdcall__)) fun2() = 0; virtual double __attribute__((__stdcall__)) fun3() = 0; }; struct mid1 : public top { virtual int __attribute__((__stdcall__)) fun1() = 0; virtual char __attribute__((__stdcall__)) fun2() = 0; virtual double __attribute__((__stdcall__)) fun3() = 0; virtual long __attribute__((__stdcall__)) fun4() = 0; }; struct mid2 : public top { virtual int __attribute__((__stdcall__)) fun1() = 0; virtual char __attribute__((__stdcall__)) fun2() = 0; virtual double __attribute__((__stdcall__)) fun3() = 0; virtual long __attribute__((__stdcall__)) fun5() = 0; }; struct mid3 : public top { virtual int __attribute__((__stdcall__)) fun1() = 0; virtual char __attribute__((__stdcall__)) fun2() = 0; virtual double __attribute__((__stdcall__)) fun3() = 0; virtual long __attribute__((__stdcall__)) fun6() = 0; }; struct bottom : public mid1, public mid2, public mid3 { int __attribute__((__stdcall__)) fun1(); char __attribute__((__stdcall__)) fun2(); double __attribute__((__stdcall__)) fun3(); long __attribute__((__stdcall__)) fun4() { return 345L; } long __attribute__((__stdcall__)) fun5() { return 456L; } long __attribute__((__stdcall__)) fun6() { return 567L; } }; int __attribute__((__stdcall__)) bottom::fun1() { return 123; } char __attribute__((__stdcall__)) bottom::fun2() { return 'a'; } double __attribute__((__stdcall__)) bottom::fun3() { return 123.45; } int main() { bottom b; b.fun1(); b.fun2(); b.fun3(); b.fun4(); b.fun5(); b.fun6(); } The command line to compile: mingw32-g++ -v -save-temps -Wall -Wextra -pedantic -o virt-test.exe virt-test .cpp And compiler output: Using built-in specs. Target: mingw32 Configured with: /usr/local/src/gcc/build-cross/source/gcc-4.1-20060331//configure -v --prefix=/usr/local/cross-mingw-4.1 --target=mingw32 --with-headers=/usr/local/cross-mingw-4.1/mingw32/include --with-gcc --with-gnu-ld --with-gnu-as --enable-threads=win32 --disable-nls --enable-languages=c,c++ --disable-win32-registry --disable-shared --enable-sjlj-exceptions --enable-libstdcxx-allocator=pool --without-newlib Thread model: win32 gcc version 4.1.1 20060331 (prerelease) /usr/local/cross-mingw-4.1/libexec/gcc/mingw32/4.1.1/cc1plus.exe -E -quiet -v virt-test.cpp -Wall -Wextra -pedantic -fpch-preprocess -o virt-test.ii ignoring nonexistent directory "/usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/../../../../mingw32/sys-include" #include "..." search starts here: #include <...> search starts here: /usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/../../../../include/c++/4.1.1 /usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/../../../../include/c++/4.1.1/mingw32 /usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/../../../../include/c++/4.1.1/backward /usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/include /usr/local/cross-mingw-4.1/lib/gcc/mingw32/4.1.1/../../../../mingw32/include End of search list. /usr/local/cross-mingw-4.1/libexec/gcc/mingw32/4.1.1/cc1plus.exe -fpreprocessed virt-test.ii -quiet -dumpbase virt-test.cpp -auxbase virt-test -Wall -Wextra -pedantic -version -o virt-test.s GNU C++ version 4.1.1 20060331 (prerelease) (mingw32) compiled by GNU C version 3.4.4 (cygming special) (gdc 0.12, using dmd 0.125). GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 Compiler executable checksum: ce3135ac5ab9fb49e72d849828fbe7dc virt-test.cpp:66: error: 'char *LTHUNK2()' aliased to undefined symbol '_ZN6bottom4fun2Ev' virt-test.cpp:66: error: 'char *LTHUNK3()' aliased to undefined symbol '_ZN6bottom4fun2Ev' virt-test.cpp:66: error: 'double *LTHUNK4()' aliased to undefined symbol '_ZN6bottom4fun3Ev' virt-test.cpp:66: error: 'double *LTHUNK5()' aliased to undefined symbol '_ZN6bottom4fun3Ev'
The DECL_ASSEMBLER_NAMES of these stdcall virtaul methods do not get decorated in time for cp/method.c:make_alias_for_thunk. (cf this comment in varasm.c: find_decl_and_mark_needed: /* We can't mark function nodes as used after cgraph global info is finished. This wouldn't generally be necessary, but C++ virtual table thunks are introduced late in the game and might seem like they need marking, although in fact they don't. */ ) Somehow we need to ensure that these methods know what their final assembler name is. This patch fixes your testcase. Could you test on your wxwidgets case? I suspect that there are cleaner ways of doing this but I don't know exactly where we can first safely modify these stdcall and fastcall names. Danny ChangeLog * target.h (asm_out.change_extern_name): New hook. * target-def.h (TARGET_ASM_CHANGE_EXTERN_NAME): New define, defaulting to hook_void_tree. Add to struct asm_out. * config/i386/cygming.h (TARGET_ASM_CHANGE_EXTERN_NAME): Override default with i386_pe_decorate_assembler_name. * config/i386/winnt.c (i386_pe_decorate_assembler_name): New function, extracted from i386_pe_encode_section_info. (i386_pe_encode_section_info): Call i386_pe_decorate_assembler_name. cp/ChangeLog * method.c (use_thunk): Call asm_out.change_extern_name before making alias for thunked-to function. Index: target.h =================================================================== --- target.h (revision 112968) +++ target.h (working copy) @@ -206,6 +206,11 @@ /* Output a DTP-relative reference to a TLS symbol. */ void (*output_dwarf_dtprel) (FILE *file, int size, rtx x); + /* This target hook allows the operating system to modify the extern assembler name + of a DECL. For example, windows targets use this to decorate stdcall and fastcall functions + with a a trailing '@n'. */ + void (*change_extern_name) (tree decl); + } asm_out; /* Functions relating to instruction scheduling. */ Index: target-def.h =================================================================== --- target-def.h (revision 112968) +++ target-def.h (working copy) @@ -221,6 +221,10 @@ #define TARGET_ASM_OUTPUT_DWARF_DTPREL NULL #endif +#ifndef TARGET_ASM_CHANGE_EXTERN_NAME +#define TARGET_ASM_CHANGE_EXTERN_NAME hook_void_tree +#endif + #define TARGET_ASM_ALIGNED_INT_OP \ {TARGET_ASM_ALIGNED_HI_OP, \ TARGET_ASM_ALIGNED_SI_OP, \ @@ -265,7 +269,8 @@ TARGET_ASM_EXTERNAL_LIBCALL, \ TARGET_ASM_MARK_DECL_PRESERVED, \ TARGET_ASM_OUTPUT_ANCHOR, \ - TARGET_ASM_OUTPUT_DWARF_DTPREL} + TARGET_ASM_OUTPUT_DWARF_DTPREL, \ + TARGET_ASM_CHANGE_EXTERN_NAME} /* Scheduler hooks. All of these default to null pointers, which haifa-sched.c looks for and handles. */ Index: config/i386/cygming.h =================================================================== --- config/i386/cygming.h (revision 112968) +++ config/i386/cygming.h (working copy) @@ -296,6 +298,7 @@ extern void i386_pe_file_end (void); extern int i386_pe_dllexport_name_p (const char *); extern int i386_pe_dllimport_name_p (const char *); +extern void i386_pe_decorate_assembler_name (tree); /* For Win32 ABI compatibility */ #undef DEFAULT_PCC_STRUCT_RETURN @@ -374,6 +377,8 @@ #define TARGET_VALID_DLLIMPORT_ATTRIBUTE_P i386_pe_valid_dllimport_attribute_p #define TARGET_CXX_ADJUST_CLASS_AT_DEFINITION i386_pe_adjust_class_at_definition +#define TARGET_ASM_CHANGE_EXTERN_NAME i386_pe_decorate_assembler_name + #undef TREE #ifndef BUFSIZ Index: config/i386/winnt.c =================================================================== --- config/i386/winnt.c (revision 112968) +++ config/i386/winnt.c (working copy) @@ -335,33 +335,38 @@ } void -i386_pe_encode_section_info (tree decl, rtx rtl, int first) +i386_pe_decorate_assembler_name (tree decl) { - default_encode_section_info (decl, rtl, first); + tree type_attributes = TYPE_ATTRIBUTES (TREE_TYPE (decl)); + tree newid = NULL_TREE; - if (first && TREE_CODE (decl) == FUNCTION_DECL) + if (lookup_attribute ("stdcall", type_attributes)) + newid = gen_stdcall_or_fastcall_suffix (decl, false); + else if (lookup_attribute ("fastcall", type_attributes)) + newid = gen_stdcall_or_fastcall_suffix (decl, true); + if (newid != NULL_TREE) { - tree type_attributes = TYPE_ATTRIBUTES (TREE_TYPE (decl)); - tree newid = NULL_TREE; + rtx rtlname = XEXP (DECL_RTL (decl), 0); + if (GET_CODE (rtlname) == MEM) + rtlname = XEXP (rtlname, 0); + XSTR (rtlname, 0) = IDENTIFIER_POINTER (newid); - if (lookup_attribute ("stdcall", type_attributes)) - newid = gen_stdcall_or_fastcall_suffix (decl, false); - else if (lookup_attribute ("fastcall", type_attributes)) - newid = gen_stdcall_or_fastcall_suffix (decl, true); - if (newid != NULL_TREE) - { - rtx rtlname = XEXP (rtl, 0); - if (GET_CODE (rtlname) == MEM) - rtlname = XEXP (rtlname, 0); - XSTR (rtlname, 0) = IDENTIFIER_POINTER (newid); - /* These attributes must be present on first declaration, - change_decl_assembler_name will warn if they are added - later and the decl has been referenced, but duplicate_decls - should catch the mismatch before this is called. */ - change_decl_assembler_name (decl, newid); - } + /* These attributes must be present on first declaration, + change_decl_assembler_name will warn if they are added + later and the decl has been referenced, but duplicate_decls + should catch the mismatch before this is called. */ + change_decl_assembler_name (decl, newid); } +} +void +i386_pe_encode_section_info (tree decl, rtx rtl, int first) +{ + default_encode_section_info (decl, rtl, first); + + if (first && TREE_CODE (decl) == FUNCTION_DECL) + i386_pe_decorate_assembler_name (decl); + /* Mark the decl so we can tell from the rtl whether the object is dllexport'd or dllimport'd. tree.c: merge_dllimport_decl_attributes handles dllexport/dllimport override semantics. */ Index: cp/method.c =================================================================== --- cp/method.c (revision 112968) +++ cp/method.c (working copy) @@ -348,13 +348,15 @@ this translation unit. */ TREE_ADDRESSABLE (function) = 1; mark_used (function); + /* The DECL_ASSEMBLER_NAME of the thunked function may need modification. */ + targetm.asm_out.change_extern_name (function); if (!emit_p) return; if (TARGET_USE_LOCAL_THUNK_ALIAS_P (function)) - alias = make_alias_for_thunk (function); + alias = make_alias_for_thunk (function); else - alias = function; + alias = function; fixed_offset = THUNK_FIXED_OFFSET (thunk_fndecl); virtual_offset = THUNK_VIRTUAL_OFFSET (thunk_fndecl);
Yes, it works like a charm now. I only built the CygWin-hosted, MinGW-targetting compiler with your patch applied, but I suppose a similar result would be achieved with a compiler bootstrapped on CygWin. I understand the patch you posted is your private copy, not applied to the official source tree yet. Do you think it'll make it for the next snapshot, so that others who are affected could use this fix? Thank you very much for your work on this, it's appreciated.
Just confirmed that the fix works with the patched CygWin-bootstrapped compiler as well. Thanks again.
*** Bug 27636 has been marked as a duplicate of this bug. ***
Confirmed based on the patch here and the dup bug.
Created attachment 12610 [details] Updated patch against the gcc-4_1-branch The posted patch works fine, it fixes compilation errors of some win32 COM code with mingw. Please commit the patch. I merged the patch by hand and created a new patch file against gcc-4_1-branch.
Created attachment 12927 [details] Updated patch against the gcc-4_2-branch
What is the status on this? I'm pretty sure this is a regression; it would be great if this fix could get into 4.2, but it might be too late for that. This is a serious problem for many/most users on Windows as it breaks COM.
Given that there's a patch here against 4.1, is this actually a regression? I recognize the seriousness of the problem, but I'm not terribly happy about the proposed patch. IIUC, the function whose name is being changed here by the proposed hook is the original method declaration, not the thunk. Why can't we arrange to have the right name when the method name is mangled?
(In reply to comment #9) > Given that there's a patch here against 4.1, is this actually a regression? > I believe it is a regression from 3.4.5 > I recognize the seriousness of the problem, but I'm not terribly happy about > the proposed patch. Nor was/am I. AFAICT, we don't know about the argument list until grokdeclarator. I am currently regtesting a patch that moves the decoration of stdcall symbol asm names from encode_section_info to a new targetm.grokdeclarator hook called just before C or C++ frontend grokdeclarator returns. Looking at dumps of MSVC objects, I note that MS does not use @n suffix for C++ stdcall symbols. Historically, g++ has, so I expect that we must continue to do so. Danny
MSC includes the calling convention as part of its C++ name mangling. Would this bug be easier to solve if the calling convention was also included as part of the C++ name mangling in GCC, instead of done afterwards? If so, it might be worth this small ABI breaking change in some future release of MinGW.
(In reply to comment #11) > MSC includes the calling convention as part of its C++ name mangling. Would > this bug be easier to solve if the calling convention was also included as part > of the C++ name mangling in GCC, instead of done afterwards? If so, it might > be worth this small ABI breaking change in some future release of MinGW. Does this bug effect Firefox and Thunderbirds XPCOM ? Aaron
Subject: Re: Compile errors with multiple inheritance where the stdcall attribute is applied to virtual functions. rridge at csclub dot uwaterloo dot ca wrote: > ------- Comment #11 from rridge at csclub dot uwaterloo dot ca 2007-05-08 12:25 ------- > MSC includes the calling convention as part of its C++ name mangling. Would > this bug be easier to solve if the calling convention was also included as part > of the C++ name mangling in GCC, instead of done afterwards? If so, it might > be worth this small ABI breaking change in some future release of MinGW. I think that would be fine, from a C++ perspective.
(In reply to comment #13) > Subject: Re: Compile errors with multiple inheritance where > the stdcall attribute is applied to virtual functions. > rridge at csclub dot uwaterloo dot ca wrote: > > ------- Comment #11 from rridge at csclub dot uwaterloo dot ca 2007-05-08 12:25 ------- > > MSC includes the calling convention as part of its C++ name mangling. Would > > this bug be easier to solve if the calling convention was also included as part > > of the C++ name mangling in GCC, instead of done afterwards? If so, it might > > be worth this small ABI breaking change in some future release of MinGW. > I think that would be fine, from a C++ perspective. If indeed XPCOM is dependant upon this then making an ABI namemangling change would break Firefox and Thunderbird addons. Aaron
(In reply to comment #13) > Subject: Re: Compile errors with multiple inheritance where > the stdcall attribute is applied to virtual functions. > > rridge at csclub dot uwaterloo dot ca wrote: > > ------- Comment #11 from rridge at csclub dot uwaterloo dot ca 2007-05-08 12:25 ------- > > MSC includes the calling convention as part of its C++ name mangling. Would > > this bug be easier to solve if the calling convention was also included as part > > of the C++ name mangling in GCC, instead of done afterwards? If so, it might > > be worth this small ABI breaking change in some future release of MinGW. > > I think that would be fine, from a C++ perspective. > Patch to this effect (without ABI breakage) at: http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01046.html and a demangled version (ie, as an attachment) of above at http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01047.html Danny
Subject: Bug 27067 Author: dannysmith Date: Thu May 24 10:11:49 2007 New Revision: 125020 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125020 Log: ChangeLog PR target/27067 * doc/tm.texi (TARGET_MANGLE_DECL_ASSEMBLER_NAME): Document. * targhooks.h (default_mangle_decl_assembler_name): Declare default hook. * targhooks.c (default_mangle_decl_assembler_name): Define default hook. * target-def.h (TARGET_MANGLE_DECL_ASSEMBLER_NAME) New. Set to default hook. * target.h (struct gcc_target): Add mangle_decl_assembler_name field. * langhooks.c (lhd_set_decl_assembler_name): Call targetm.mangle_decl_assembler_name for names with global scope. * config/i386/cygming.h (TARGET_MANGLE_DECL_ASSEMBLER_NAME) Override default. (ASM_OUTPUT_DEF_FROM_DECLS): Simplify to use DECL_ASSEMBLER_NAME. * config/i386/i386-protos.h (i386_pe_mangle_decl_assembler_name): Declare. * config/i386/winnt.c (i386_pe_maybe_mangle_decl_assembler_name): New. Factored out of i386_pe_encode_section_info. (gen_stdcall_or_fastcall_suffix): Get name identifier as argument. Move check for prior decoration of stdcall symbols to i386_pe_encode_section_info. (i386_pe_encode_section_info): Adjust call to gen_stdcall_or_fastcall_suffix. Use i386_pe_maybe_mangle_decl_assembler_name, if needed. (i386_pe_mangle_decl_assembler_name): New. Wrap i386_pe_maybe_mangle_decl_assembler_name. cp/ChangeLog * mangle.c (mangle_decl): Call targetm.mangle_decl_assembler_name. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/cygming.h trunk/gcc/config/i386/i386-protos.h trunk/gcc/config/i386/winnt.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/mangle.c trunk/gcc/doc/tm.texi trunk/gcc/langhooks.c trunk/gcc/target-def.h trunk/gcc/target.h trunk/gcc/targhooks.c trunk/gcc/targhooks.h
Fixed on trunk