Bug 16030 - [3.4 Regression] [cygwin/mingw]: stdcall function decoration vs LTHUNK alias in multiple inheritanc
Summary: [3.4 Regression] [cygwin/mingw]: stdcall function decoration vs LTHUNK alias ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.4.1
: P2 normal
Target Milestone: 4.0.0
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: patch, wrong-code
Depends on:
Blocks:
 
Reported: 2004-06-17 02:53 UTC by Danny Smith
Modified: 2006-02-28 09:28 UTC (History)
2 users (show)

See Also:
Host: i686-pc-mingw32
Target: i686-pc-mingw32
Build: i686-pc-mingw32
Known to work: 4.0.0
Known to fail: 3.4.1
Last reconfirmed: 2006-01-06 15:25:51


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Danny Smith 2004-06-17 02:53:27 UTC
The  decoration of stdcall symbols in winnt.c:i386_pe_encode_section_info
causesa problems with the use of static aliases for MI thunks in
cp/method.c 

This problem is in 3.4.1 and trunk.

Consider this testcase:

// stdcall_thunk.C
class  MBase
{
public:
  virtual int __attribute__((stdcall)) vf() const = 0;
  virtual ~MBase() {};
};

class D1 : virtual public MBase
{
public:
  int __attribute__((stdcall)) vf() const;
};

class M1: public D1
{
public:
  int __attribute__((stdcall)) vf() const;
};

int D1::vf() const { return 1; }
int M1::vf() const { return D1::vf();}


This produces the assembly:

	.file	"stdcall_thunk.C"
	.def	LTHUNK0@4;	.scl	3;	.type	32;	.endef
	.set	LTHUNK0@4,__ZNK2D12vfEv  <<< not decorated

[...]

.globl __ZNK2D12vfEv@4  <<< stdcall decoration
	.def	__ZNK2D12vfEv@4;	.scl	2;	.type	32;	.endef
__ZNK2D12vfEv@4: 
	pushl	%ebp
	movl	%esp, %ebp
	movl	$1, %eax
	leave
	ret	$4
[...]


Note that although the D1::vf name has the correct stdcall decoaration,
the target of LTHUNK0@4 does not.

Although ld actually has an option to fix-up stdcall symbols to
the undecorated name, using that option can mask stack corruption bugs
until the app is actually run.

One way to fix is to use the RTL name rather than DECL_ASSEMBLER_NAME in
make alias_for_thunk, ie:

Index: method.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
retrieving revision 1.285
diff -c -3 -p -r1.285 method.c
*** method.c	16 Jun 2004 01:21:31 -0000	1.285
--- method.c	17 Jun 2004 02:41:09 -0000
*************** make_alias_for_thunk (tree function)
*** 314,320 ****
    SET_DECL_ASSEMBLER_NAME (alias, DECL_NAME (alias));
    TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (alias)) = 1;
    if (!flag_syntax_only)
!     assemble_alias (alias, DECL_ASSEMBLER_NAME (function));
    return alias;
  }
  
--- 314,330 ----
    SET_DECL_ASSEMBLER_NAME (alias, DECL_NAME (alias));
    TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (alias)) = 1;
    if (!flag_syntax_only)
!     {
!       /* Using DECL_ASSEMBLER_NAME to get the identifier for the alias
! 	 target loses any decoration that targetm.encode_section_info
! 	 may have added.  Use the RTL name instead.  */
!       tree idp;
!       /* Is this necessary?  */
!       if (!DECL_RTL_SET_P (function))
! 	make_decl_rtl (function, NULL);
!       idp = get_identifier (XSTR (XEXP (DECL_RTL (function), 0), 0));
!       assemble_alias (alias, idp);
!     }
    return alias;
  }
  


With above patch, I get this:

	.file	"stdcall_thunk.C"
	.def	LTHUNK0@4;	.scl	3;	.type	32;	.endef
	.set	LTHUNK0@4,__ZNK2D12vfEv@4  <<< OK

[...]

.globl __ZNK2D12vfEv@4
	.def	__ZNK2D12vfEv@4;	.scl	2;	.type	32;	.endef
__ZNK2D12vfEv@4:
	pushl	%ebp
	movl	%esp, %ebp
	movl	$1, %eax
	leave
	ret	$4


Is there a better way to do this?

Danny
Comment 1 Danny Smith 2004-07-07 01:04:03 UTC
Patch submitted:
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00534.html
Comment 2 Andrew Pinski 2004-07-07 01:07:17 UTC
Since LTHUNK is new, this is a regression.
Confirmed.
Comment 3 Steven Bosscher 2004-08-12 08:15:34 UTC
Mark, can you review the patch at 
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00534.html? 
 
Comment 4 Mark Mitchell 2004-08-12 14:35:25 UTC
Subject: Re:  [3.4/3.5 Regression] [cygwin/mingw]: stdcall
 function decoration vs LTHUNK alias in multiple inheritanc

steven at gcc dot gnu dot org wrote:

>------- Additional Comments From steven at gcc dot gnu dot org  2004-08-12 08:15 -------
>Mark, can you review the patch at 
>http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00534.html? 
>  
>
As a minor point, the get_identifier bit of that patch is very 
inefficient.  Instead, assemble_alias should be modified to take a "char 
*" argument, or a new function should be created that both 
assemble_alias and make_alias_fr_thunk can use.

But, a more major point is that I'm not at all confident that it's 
correct to use the encoded name on all targets.  Maybe 
ASM_OUTPUT_DEF_FROM_DECLS should be defined for these targets instead?

Comment 5 Danny Smith 2004-08-18 10:06:51 UTC
cygming.h already does define ASM_OUTPUT_DEF_FROM_DECLS and it uses the RTL
encoded name for the new decl, but leaves the target of the alias alone. I don't
think it should modify the target,(at least in __attribute__ ((alias
("target"))) usage).

New patch submitted at:
http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01320.html

Danny
Comment 6 Mark Mitchell 2004-08-19 21:21:29 UTC
Patch OK, please commit.
Comment 7 GCC Commits 2004-08-21 08:02:12 UTC
Subject: Bug 16030

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	dannysmith@gcc.gnu.org	2004-08-21 08:02:04

Modified files:
	gcc            : ChangeLog 
	gcc/config/i386: winnt.c 

Log message:
	PR  c++/16030
	* config/i386/winnt/c (gen_stdcall_suffix, gen_fastcall_suffix):
	Remove, merging into ...
	(gen_stdcall_or_fastcall_suffix): New function, returning tree
	rather than const char*, and accepting additional parameter.
	Don't add suffix to '*'-prefixed symbols or variadic functions.
	(i386_pe_encode_section_info): Adjust for call to new function.
	Call change_decl_assembler_name.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.5013&r2=2.5014
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/winnt.c.diff?cvsroot=gcc&r1=1.70&r2=1.71

Comment 8 Danny Smith 2004-08-21 08:05:21 UTC
Fixed on 3.5  The patch applies cleanly to 3.4.2.  OK there as well?
Danny
Comment 9 Mark Mitchell 2004-08-23 06:54:00 UTC
Subject: Re:  [3.4 Regression] [cygwin/mingw]: stdcall function
 decoration vs LTHUNK alias in multiple inheritanc

dannysmith at users dot sourceforge dot net wrote:

>------- Additional Comments From dannysmith at users dot sourceforge dot net  2004-08-21 08:05 -------
>Fixed on 3.5  The patch applies cleanly to 3.4.2.  OK there as well?
>Danny
>
>  
>
OK.

Comment 10 Mark Mitchell 2004-08-29 18:51:57 UTC
Postponed until GCC 3.4.3.
Comment 11 Mark Mitchell 2004-10-31 02:01:39 UTC
Postponed until GCC 3.4.4.  

Although it's somewhat sad the patch has not yet been applied, given that I
approved it in August.  It's still OK to apply the patch, before 3.4.3, if
somebody wants to do that.
Comment 12 Giovanni Bajo 2004-10-31 02:05:34 UTC
Danny, can you please quickly retest the patch and apply it immediatly to the 
branch? We might be still in time for 3.4.3.
Comment 13 Danny Smith 2004-10-31 02:37:06 UTC
No, I won't backport this patch.  Although the patch did apply cleanly and 
fixed this bug,  using change_decl_assembler name caused warnings with mingw 
(in my code base, not in testsuite) that are not seen on trunk.  I'm sorry that 
I didn't report this immediately.  I'll try to sort it out before 3.4.4 but I 
do not see this as a high priority problem.

Danny
Comment 14 Gabriel Dos Reis 2005-10-07 03:23:33 UTC
Not a high priority as per maintainer.  Postponed to 3.4.6 era
Comment 15 Gabriel Dos Reis 2006-02-28 09:28:35 UTC
Fixed in 4.0 and up.