[Bug c++/55171] New: incorrect virtual thunk on mingw

[Bug c++/55171] New: incorrect virtual thunk on mingw

[mitza at ociweb dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

             Bug #: 55171
           Summary: incorrect virtual thunk on mingw
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mitza@ociweb.com


Created attachment 28593
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28593
test for virtual call on mingw

When a virtual call invoked on an object which uses both virtual and
non-virtual inheritance, the virtual thunk leaves the target function with an
invalid "this" pointer.

Attached test fails with output:

created Derived object at: 0x2c1a50
Derived virtual call at: 0x409800
Assertion failed!

>From gdb:
In the body of the most-derived constructor
(gdb) inf vt this
vtable for 'Derived' @ 0x4097ec (subobject @ 0x2c1a50):
[0]: 0x40188a <Derived::~Derived()>
[1]: 0x40190a <Derived::~Derived()>
[2]: 0x401932 <Derived::foo() const>

vtable for 'VirtBase' @ 0x409808 (subobject @ 0x2c1a58):
[0]: 0x401902 <virtual thunk to Derived::~Derived()>
[1]: 0x40192a <virtual thunk to Derived::~Derived()>
[2]: 0x4019d1 <virtual thunk to Derived::foo() const>

0x004019d1 in virtual thunk to Derived::foo() const () at gccbug.cpp:45
Dump of assembler code for function _ZTv0_n16_NK7Derived3fooEv:
=> 0x004019d1 <+0>:     mov    0x4(%esp),%eax
   0x004019d5 <+4>:     mov    (%eax),%eax
   0x004019d7 <+6>:     add    -0x10(%eax),%eax
   0x004019da <+9>:     mov    %eax,0x4(%esp)
   0x004019de <+13>:    jmp    0x401932 <Derived::foo() const>

After thunk has jumped to target function
Derived::foo (this=0x409800) at gccbug.cpp:41
41      {
(gdb) inf vt this
Cannot access memory at address 0xffffffec

[Bug c++/55171] incorrect virtual thunk on mingw

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cgf at gcc dot gnu.org,
                   |                            |dave.korn.cygwin at gmail
                   |                            |dot com, ktietz at gcc dot
                   |                            |gnu.org

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-11-02 11:13:16 UTC ---
Thus, is this mingw specific? Should be target instead?

[Bug c++/55171] incorrect virtual thunk on mingw

[mitza at ociweb dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #2 from Adam Mitz <mitza at ociweb dot com> 2012-11-02 19:51:26 UTC ---
(In reply to comment #1)
> Thus, is this mingw specific?

I can only get it to fail on mingw.


> Should be target instead?

Sorry, I don't know what this means.

[Bug target/55171] incorrect virtual thunk on mingw

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |target

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-11-02 20:13:21 UTC ---
Means this, easy ;)

[Bug target/55171] incorrect virtual thunk on mingw

[mitza at ociweb dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #4 from Adam Mitz <mitza at ociweb dot com> 2012-11-07 21:10:26 UTC ---
The generated code for the thunk is incorrect in that it re-uses the %eax
register, clobbering the original "this" value:

Dump of assembler code for function _ZTv0_n16_NK7Derived3fooEv:
=> 0x004019d1 <+0>:     mov    0x4(%esp),%eax
   0x004019d5 <+4>:     mov    (%eax),%eax
   0x004019d7 <+6>:     add    -0x10(%eax),%eax
   0x004019da <+9>:     mov    %eax,0x4(%esp)
   0x004019de <+13>:    jmp    0x401932 <Derived::foo() const>

vtable = *base_this
derived_this = vtable + *(vtable - 16)

The same thunk on Linux uses both %eax and %ecx:

Dump of assembler code for function _ZTv0_n16_NK7Derived3fooEv:
=> 0x08048eab <+0>:     mov    0x8(%esp),%eax
   0x08048eaf <+4>:     mov    (%eax),%ecx
   0x08048eb1 <+6>:     add    -0x10(%ecx),%eax
   0x08048eb4 <+9>:     mov    %eax,0x8(%esp)
   0x08048eb8 <+13>:    jmp    0x8048e08 <Derived::foo() const>

vtable = *base_this
derived_this = base_this + (*vtable - 16)

The MinGW version must either clobber a scratch register like %ecx or %edx, or
if this isn't possible (fastcall/thiscall?), save to the stack and restore.

[Bug target/55171] incorrect virtual thunk on mingw

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #5 from Mikael Pettersson <mikpe at it dot uu.se> 2012-11-11 14:30:14 UTC ---
It's caused by "[patch i386]: Set for method-functions default
calling-convention to thiscall for 32-bit msabi" in r171890:
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00066.html
http://gcc.gnu.org/ml/gcc-cvs/2011-04/msg00082.html

With x86_64-w64-mingw32-g++ -m32 -O2 there are numerous code generation
differences in r171890 for this test case (as expected with the changed calling
convention), but looking only at the virtual thunk we see:

--- pr55171.s-r171889   2012-11-11 14:51:38.000000000 +0100
+++ pr55171.s-r171890   2012-11-11 15:00:58.000000000 +0100
...
 __ZTv0_n16_NK7Derived3fooEv:
-       movl    8(%esp), %eax
-       movl    (%eax), %ecx
-       addl    -16(%ecx), %eax
-       movl    %eax, 8(%esp)
+       movl    4(%esp), %eax
+       movl    (%eax), %eax
+       addl    -16(%eax), %eax
+       movl    %eax, 4(%esp)
        jmp     LTHUNK2
...

The two code sequences are clearly not equivalent (ignore the stack position of
the pointer being adjusted).

Running the test case confirms: with g++ @ r171889 it succeeds, but with g++ @
r171890 it gets a runtime failure.

[Bug target/55171] incorrect virtual thunk on mingw

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wriabi at email dot com

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-11-23 21:57:11 UTC ---
*** Bug 55367 has been marked as a duplicate of this bug. ***

[Bug c++/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-11-24
     Ever Confirmed|0                           |1

--- Comment #7 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-11-24 12:47:37 UTC ---
Yes, the issue here is the generation of patterns - like

movl    (%ecx), %eax
addl    -12(%eax), %ecx

- for thunks.

The following patch solves this issue for me.

Index: i386.c
===================================================================
--- i386.c    (Revision 193669)
+++ i386.c    (Arbeitskopie)
@@ -9567,6 +9567,8 @@
       tree decl = current_function_decl, fntype = TREE_TYPE (decl);
       bool fastcall_p
     = lookup_attribute ("fastcall", TYPE_ATTRIBUTES (fntype)) != NULL_TREE;
+      bool thiscall_p
+    = lookup_attribute ("thiscall", TYPE_ATTRIBUTES (fntype)) != NULL_TREE;
       bool static_chain_p = DECL_STATIC_CHAIN (decl);
       int regparm = ix86_function_regparm (fntype, decl);
       int drap_regno
@@ -9577,10 +9579,13 @@
       if ((regparm < 1 || (fastcall_p && !static_chain_p))
       && drap_regno != AX_REG)
     regno = AX_REG;
-      else if (regparm < 2 && drap_regno != DX_REG)
+      else if (thiscall_p
+                 && !static_chain_p && drap_regno != DX_REG)
+          regno = DX_REG;
+      else if (regparm < 2 && !thiscall_p && drap_regno != DX_REG)
     regno = DX_REG;
       /* ecx is the static chain register.  */
-      else if (regparm < 3 && !fastcall_p && !static_chain_p
+      else if (regparm < 3 && !fastcall_p && !thiscall_p && !static_chain_p
            && drap_regno != CX_REG)
     regno = CX_REG;
       else if (ix86_save_reg (BX_REG, true))
@@ -11092,12 +11097,15 @@
     return R11_REG;
   else
     {
-      bool is_fastcall;
+      bool is_fastcall, is_thiscall;
       int regparm;

       is_fastcall = (lookup_attribute ("fastcall",
                        TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
              != NULL);
+      is_thiscall = (lookup_attribute ("thiscall",
+                       TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+             != NULL);
       regparm = ix86_function_regparm (TREE_TYPE (cfun->decl), cfun->decl);

       if (is_fastcall)
@@ -11110,6 +11118,12 @@
         }
       return AX_REG;
     }
+      else if (is_thiscall)
+        {
+      if (!DECL_STATIC_CHAIN (cfun->decl))
+        return DX_REG;
+      return AX_REG;
+    }
       else if (regparm < 3)
     {
       if (!DECL_STATIC_CHAIN (cfun->decl))
@@ -25046,7 +25060,7 @@

       fntype = TREE_TYPE (fndecl);
       ccvt = ix86_get_callcvt (fntype);
-      if ((ccvt & (IX86_CALLCVT_FASTCALL | IX86_CALLCVT_THISCALL)) != 0)
+      if ((ccvt & IX86_CALLCVT_FASTCALL) != 0)
     {
       /* Fastcall functions use ecx/edx for arguments, which leaves
          us with EAX for the static chain.
@@ -25054,6 +25068,8 @@
          leaves us with EAX for the static chain.  */
       regno = AX_REG;
     }
+      else if ((ccvt & IX86_CALLCVT_THISCALL) != 0)
+        regno = DX_REG;
       else if (ix86_function_regparm (fntype, fndecl) == 3)
     {
       /* For regparm 3, we have no free call-clobbered registers in
@@ -34713,8 +34729,10 @@
   else
     {
       unsigned int ccvt = ix86_get_callcvt (TREE_TYPE (function));
-      if ((ccvt & (IX86_CALLCVT_FASTCALL | IX86_CALLCVT_THISCALL)) != 0)
+      if ((ccvt & IX86_CALLCVT_FASTCALL) != 0)
     tmp_regno = AX_REG;
+      else if ((ccvt & IX86_CALLCVT_THISCALL) != 0)
+        tmp_regno = DX_REG;
       else
     tmp_regno = CX_REG;
     }

[Bug c++/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #8 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-11-24 12:54:40 UTC ---
The pattern is

movl    4(%esp), %eax
movl    (%eax), %eax
addl    -16(%eax), %eax

As here the saved this-pointer in %eax gets clobbered.

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.3

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #9 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-11-29 09:18:59 UTC ---
Author: ktietz
Date: Thu Nov 29 09:18:54 2012
New Revision: 193925

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193925
Log:
        PR target/55171
        * i386.c (get_scratch_register_on_entry): Handle
        thiscall-convention.
        (split_stack_prologue_scratch_regno): Likewise.
        (ix86_static_chain): Likewise.
        (x86_output_mi_thunk): Likewise.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/i386/i386.c

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #10 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-11-29 09:21:23 UTC ---
Author: ktietz
Date: Thu Nov 29 09:21:17 2012
New Revision: 193926

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193926
Log:
        PR target/55171
        * i386.c (get_scratch_register_on_entry): Handle
        thiscall-convention.
        (split_stack_prologue_scratch_regno): Likewise.
        (ix86_static_chain): Likewise.
        (x86_output_mi_thunk): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-11-29 09:25:01 UTC ---
Fixed

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[steve.k.chiu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

Steve K. Chiu <steve.k.chiu at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |steve.k.chiu at gmail dot
                   |                            |com

--- Comment #12 from Steve K. Chiu <steve.k.chiu at gmail dot com> 2013-05-06 01:10:23 UTC ---
ffmpeg reports wrong duration after this patch. ffmpeg (official 1.2 release)
is compiled with x32-4.7.2-release-win32-sjlj-rev11.7z from mingw-builds, and
the test media file is a 5.18G recorded tv show (in mpegts format). 

While the same code and test media report the correct duration 1:00:xx with
x32-4.7.2-release-win32-sjlj-rev10.7z (without Bug 55171 patch), with
x32-4.7.2-release-win32-sjlj-rev11.7z (with Bug 55171 patch) report the wrong
duration 00:13:xx.

The media file is too large to upload anywhere, but I suspect any large mpeg2ts
file may have the same issues.

[Bug target/55171] [4.7/4.8 Regression] incorrect virtual thunk on mingw

[i.nixman at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55171

--- Comment #13 from niXman <i.nixman at gmail dot com> 2013-05-06 22:31:01 UTC ---
Kai, any ideas?