[Bug c++/55910] New: One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[Bug c++/55910] New: One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[oder at eleks dot lviv.ua]

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

             Bug #: 55910
           Summary: One of instances generated for a method with -O3 uses
                    incorrect parameter cleanup size with 'ret'
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: oder@eleks.lviv.ua


Created attachment 29108
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29108
Method source and two its instances disassembled.

The bug was initially reported by me for MinGW x86, however they told me there
to fill it for GCC.

After upgrading to GCC 4.7.2 I'having an issue that release build of my DLL
crashes. Debugging shows that this is due to bad code generated by GCC, Alas I
can't provide small test case but here is the faulting function fragment
itself.
 -----------------------
 int CProfileGenerator::CalcAccProfile(
 pos_type p0,
 vel_type v0,
 acc_type a0,
 jerk_type j,
 vel_type vel,
 acc_type acc,
 CProfile& ret_profile
 ) const {
 time_type t0 = 0.0;
 ret_profile.clear(); ret_profile.reserve(10);

 vel_type dv = vel-v0;

 if (ABS(dv) < m_epsilon_v && ABS(a0) < m_epsilon_a) {
 if (dv == 0.0) {
 ret_profile.push_back(CProfileStep(0.0, p0, v0, a0, 0.0));
VALIDATEPROFILE(ret_profile);
 } else {
 acc_type a0 = (dv<0.0)?-1.0:1.0;
 time_type dt = ABS(dv);
 pos_type p1 = p0 + v0*dt + 0.5*dv*dv;
 ret_profile.push_back(CProfileStep(0.0, p0, v0, a0, 0.0));
VALIDATEPROFILE(ret_profile);
 ret_profile.push_back(CProfileStep(dt, p1, vel, 0.0, 0.0));
VALIDATEPROFILE(ret_profile);
 }
 G_BLOG_4("acc profile calc skiped ");
 return ret_profile.size();
 }
 if (v0 < vel) {
 acc = ABS(acc);
 } else if (v0 == vel) {
 acc = -SIGN(a0)*ABS(acc);
 } else {
 acc = -ABS(acc);
 }

 if (j == 0.0) {
 return InternalAccCalc(t0, p0, v0, 0.0, 0.0, true, true, vel, acc, true,
ret_profile);
 // <------------------------------ FAULTS DUE TO RETURNING TO ADDRESS
0x00000000 HERE
 }
 ...
 -----------------------
 Here all the *_type typedefs are doubles and CProfile is std::vector of
CProfileStep structure.

 I've built library with -fno-inline to be able to see function calls more
clearly and here is the problem is assembler code.
 -----------------------
 Dump of assembler code for function CProfileGenerator::CalcAccProfile(double,
double, double, double, double, double, CP
 rofileGenerator::CProfile&) const:
 0x100622e0 <+0>: push %ebp
 0x100622e1 <+1>: mov %ecx,%ebp
 0x100622e3 <+3>: push %edi
 0x100622e4 <+4>: push %esi
 0x100622e5 <+5>: push %ebx
 0x100622e6 <+6>: sub $0x14c,%esp
 ; <------------------------ Initial stack preparation ends here
 0x100622ec <+12>: mov 0x160(%esp),%eax
 0x100622f3 <+19>: mov 0x190(%esp),%ebx
 0x100622fa <+26>: fldl 0x188(%esp)
 0x10062301 <+33>: fstpl 0x98(%esp)
 ...
 ...
 ...
 0x1006254a <+618>: jbe 0x10062920 <CProfileGenerator::CalcAccProfile(double,
double, double, double, double, dou
 ble, CProfileGenerator::CProfile&) const+1600>
 0x10062550 <+624>: fldl 0x98(%esp)
 0x10062557 <+631>: fstpl (%esp)
 0x1006255a <+634>: call 0x1005f2e0 <ABS<double>(double const&)>
 0x1006255f <+639>: fldl 0xb0(%esp)
 0x10062566 <+646>: fldz
 0x10062568 <+648>: fld %st(0)
 0x1006256a <+650>: fxch %st(2)
 0x1006256c <+652>: fucomi %st(2),%st
 0x1006256e <+654>: fstp %st(2)
 0x10062570 <+656>: jnp 0x10062b00 <CProfileGenerator::CalcAccProfile(double,
double, double, double, double, dou
 ble, CProfileGenerator::CProfile&) const+2080>
 ...
 ...
 ...
 0x10062b00 <+2080>: jne 0x10062580 <CProfileGenerator::CalcAccProfile(double,
double, double, double, double, dou
 ble, CProfileGenerator::CProfile&) const+672>
 0x10062b06 <+2086>: fstp %st(1)
 0x10062b08 <+2088>: fxch %st(1)
 0x10062b0a <+2090>: fstpl 0x30(%esp)
 0x10062b0e <+2094>: mov %ebx,%ecx
 0x10062b10 <+2096>: fldl 0x90(%esp)
 0x10062b17 <+2103>: mov $0x1,%edx
 0x10062b1c <+2108>: mov $0x1,%eax
 0x10062b21 <+2113>: fstpl 0x28(%esp)
 0x10062b25 <+2117>: fstl 0x20(%esp)
 0x10062b29 <+2121>: fstl 0x18(%esp)
 0x10062b2d <+2125>: fldl 0x88(%esp)
 0x10062b34 <+2132>: fstpl 0x10(%esp)
 0x10062b38 <+2136>: fldl 0xc8(%esp)
 0x10062b3f <+2143>: fstpl 0x8(%esp)
 0x10062b43 <+2147>: fstpl (%esp)
 0x10062b46 <+2150>: call 0x10061890
<CProfileGenerator::InternalAccCalc(double, double, double, double, double, bo
 ol, bool, double, double, bool, CProfileGenerator::CProfile&) const>

 0x10062b4b <+2155>: sub $0x38,%esp <-----------------------------------------
THIS COMMAND CORRUPTS STACK POINTER WHICH OTHERWISE WOULD BE VALID

 0x10062b4e <+2158>: add $0x14c,%esp
 0x10062b54 <+2164>: pop %ebx
 0x10062b55 <+2165>: pop %esi
 0x10062b56 <+2166>: pop %edi
 0x10062b57 <+2167>: pop %ebp
 0x10062b58 <+2168>: ret $0x34
 -----------------------

It looks like you reuse the same stack area for all the calls and just
re-adjust stack pointer back to compensate ret <N> instruction in functions
being called. With this, the cause of the problem is the following. It's all
about the method CProfileGenerator::InternalAccCalc which is being invoked. In
function faulting the method is called 4 times from different execution paths
(the first one shown above and the other 3 further in the function). Actually,
GCC has generated two instances of CProfileGenerator::InternalAccCalc: at
0x1005aa40 and 0x1005a660, - each of those two instances being called twice.
For first instance the stack is adjusted by 0x38 in caller, while for second
instance the stack is adjusted by 0x48. However if second instance contains
correct "ret 0x48" exit instructions, the first one containg just plain "ret"
which causes the stack corruption.
 Attached is the method's source and disassembled text of two instances
generated.
 The instances are invoked as InternalAccCalc(t0, p0, v0, 0.0, 0.0, true, true,
vel, acc, true, ret_profile); // #1
 InternalAccCalc(t0, p0, v0, a0, j, z1, z2, vel, acc, true, ret_profile); // #1
 InternalAccCalc(t0, p0, v0, a0, j, z1, z2, vel, limit_acc1, false,
ret_profile); // #2
 InternalAccCalc(t0, p0, v0, a0, j, z1, z2, vel, limit_acc2, false,
ret_profile); // #2




The GCC was auto-installed by setup application from SourceForge
 H:>"c:\Util\MinGW\bin\g++.exe" -v
 Using built-in specs.
 COLLECT_GCC=c:\Util\MinGW\bin\g++.exe

COLLECT_LTO_WRAPPER=c:/util/mingw/bin/../libexec/gcc/mingw32/4.7.2/lto-wrapper.e
 xe
 Target: mingw32
 Configured with: ../gcc-4.7.2/configure
--enable-languages=c,c++,ada,fortran,obj
 c,obj-c++ --disable-sjlj-exceptions --with-dwarf2 --enable-shared
--enable-libgo
 mp --disable-win32-registry --enable-libstdcxx-debug
--disable-build-poststage1-
 with-cxx --enable-version-specific-runtime-libs --build=mingw32
--prefix=/mingw
 Thread model: win32
 gcc version 4.7.2 (GCC)

 The file was compiled with following options
 g++ -D"USED_LIBS=" -D_NDEBUG -DNDEBUG -DNODEBUG -DNO_EXCEPTIONS
-D__STL_THREADS
 -D_MT -DUSE_DELPHI_STRINGS -mwindows -mconsole -mthreads -malign-double -Wall
-
 W -Wno-unused-function -Wno-unused-parameter -O3 -march=i686 -g -DLOGING -D_C
 ONSOLE -fno-exceptions -fno-rtti -Woverloaded-virtual -fno-inline
../../shared/pr
 ofiles/MoveProfileGenerator.cpp -c -o bin/release/MoveProfileGenerator.obj

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[paolo.carlini at oracle dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz at gcc dot gnu.org

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-01-08 17:45:53 UTC ---
CC-ing Kai.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[paolo.carlini at oracle dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-01-08
     Ever Confirmed|0                           |1

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-01-08 17:47:04 UTC ---
Note, a self-contained testcase is mandatory per the Bug Reporting
instructions.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[oder at eleks dot lviv.ua]

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

--- Comment #3 from Oleh Derevenko <oder at eleks dot lviv.ua> 2013-01-08 17:51:59 UTC ---
Well, this is an optimizer issue which goes away with -O2. And optimizer is
something that is hard to predict and reproduce, when you run over an issue in
complex code with lots of dependencies and inlined methods.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[paolo.carlini at oracle dot com]

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

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-01-08 17:57:28 UTC ---
Indeed, and that's exactly why we need a precise, specific testcase, no hand
waving.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[oder at eleks dot lviv.ua]

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

--- Comment #5 from Oleh Derevenko <oder at eleks dot lviv.ua> 2013-01-08 18:11:42 UTC ---
IMHO, I've localized the issue clearly enough. The code that generates
instances for methods may sometimes consider some of those instances as having
no parameters in method instance itself and as having some nonzero size of
parameters (namely 0x38) in caller that re-adjusts stack after method instance
has cleared the parameters.
If those numbers would be taken from single variable that kind of bug would be
impossible. So please examine the code and see where do you get total parameter
size for 'ret' instruction from and where do you get total parameter size for
stack re-adjustment from. And then please try to realize why those locations
could possibly have different numbers in them.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-01-09 09:55:35 UTC ---
> IMHO, I've localized the issue clearly enough. The code that generates
> instances for methods may sometimes consider some of those instances as having
> no parameters in method instance itself and as having some nonzero size of
> parameters (namely 0x38) in caller that re-adjusts stack after method instance
> has cleared the parameters.
> If those numbers would be taken from single variable that kind of bug would be
> impossible. So please examine the code and see where do you get total parameter
> size for 'ret' instruction from and where do you get total parameter size for
> stack re-adjustment from. And then please try to realize why those locations
> could possibly have different numbers in them.

When -O2 is enabled, there are dozens of different passes massaging the initial
code and trying to optimize it.  It is essentially impossible to predict how
they interact with each other given the complexity explosion, that's why being
able to replay the compilation is required.

The rules are clear: if you want the GCC team to look at your problem, you must
provide a reproducer.  Otherwise this PR will be closed as WONTFIX.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[oder at eleks dot lviv.ua]

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

--- Comment #7 from Oleh Derevenko <oder at eleks dot lviv.ua> 2013-01-09 17:26:37 UTC ---
OK. If you are unable to track where you get size of function parameter block
from, close it. I have much other work to do, rather than trying to reproduce
your bugs.

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[oder at eleks dot lviv.ua]

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

--- Comment #8 from Oleh Derevenko <oder at eleks dot lviv.ua> 2013-01-09 17:42:19 UTC ---
And go on telling your tales about numerous optimizations that affect argument
count on stack and cleanup size of final 'ret'...

[Bug c++/55910] One of instances generated for a method with -O3 uses incorrect parameter cleanup size with 'ret'

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |WONTFIX

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-01-09 19:23:31 UTC ---
> And go on telling your tales about numerous optimizations that affect argument
> count on stack and cleanup size of final 'ret'...

Most people get it though, not clear why you don't.  Anyway, you seems to have
a simple workaround (compiling with -O2 instead of -O3) so you aren't blocked.