Bug 1122448 - invalid code generated by g++4.4.7-4
Summary: invalid code generated by g++4.4.7-4
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: gcc
Version: 6.5
Hardware: x86_64
OS: Linux
Target Milestone: rc
: ---
Assignee: Jakub Jelinek
QA Contact: qe-baseos-tools
Depends On:
TreeView+ depends on / blocked
Reported: 2014-07-23 09:43 UTC by Jakub Zytka
Modified: 2014-07-28 08:34 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2014-07-28 08:34:50 UTC

Attachments (Terms of Use)
test case exposing problem (1.72 KB, text/x-c++src)
2014-07-23 09:43 UTC, Jakub Zytka
no flags Details
makefile (250 bytes, text/plain)
2014-07-23 09:43 UTC, Jakub Zytka
no flags Details
generated assembly (79.30 KB, text/plain)
2014-07-23 09:44 UTC, Jakub Zytka
no flags Details
test case exposing problem (1.75 KB, text/x-c++src)
2014-07-23 09:57 UTC, Jakub Zytka
no flags Details
generated assembly (79.07 KB, text/plain)
2014-07-23 09:58 UTC, Jakub Zytka
no flags Details

Description Jakub Zytka 2014-07-23 09:43:20 UTC
Created attachment 920180 [details]
test case exposing problem

Description of problem:

Gcc generates code that reuses stack for two different variables, which are in different scopes. Unfortunately, modification of the already out-of-scope variable happens after initialization of the second variable, effectively corrupting memory.

I attach cpp reproducing the issue; the idea is as follows:
create corruptingVar
create corruptedVar // by default initalized with 0s; tmp62 in the assembly
check correct initialization;

Lets look at the generated assembly:

        pxor    %xmm0, %xmm0    # tmp62
        movdqa  %xmm0, (%rsp)   # tmp62,
        movq    $_ZTV11TripleArray+16, (%rsp)   #, corruptingVar.D.3281._vptr.TripleArray
        movdqa  %xmm0, 16(%rsp) # tmp62,
        movdqa  %xmm0, 32(%rsp) # tmp62,
        movdqa  %xmm0, 48(%rsp) # tmp62,
        cmpq    $0, (%rsp)      #, corruptedVar.data
        jne     .L28    #,

Apparently, in the middle of tmp62 initialization (top of stack) a pointer to vtable is set for already out-of-scope variable, which corrupts tmp62.
Version-Release number of selected component (if applicable):
g++ (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)

How reproducible:

I attach cpp code + makefile reproducing the issue. It is probably not minimal, but I believe it is short enough.
I also attach assembly as generated on my machine.

Steps to Reproduce:
1. make
2. ./test

Actual results:
process aborts

Expected results:
no error

Additional info:
The compiler memory barrier after ccBeingCorrupted is added to make reproduction test case smaller. Same goes with the noinline attribute.

I'm compiling with -fno-strict-aliasing because the original code requires it. The reproduction test doesn't do anything fishy, but once -fno-strict-aliasing is removed the problem doesn't reproduce on provided test case.

As far as I understand -O3 -fno-strict-aliasing is a legitimate, fully supported combination. Please correct me if I'm wrong.

The problem reproduces on g++-4.1 (GCC) 4.1.2 20080704 (Red Hat 4.1.2-52)

The test case doesn't reproduce the problem on g++-4.9.0. I haven't tried hard to reproduce it though.

Comment 1 Jakub Zytka 2014-07-23 09:43:57 UTC
Created attachment 920181 [details]

Comment 2 Jakub Zytka 2014-07-23 09:44:44 UTC
Created attachment 920182 [details]
generated assembly

Comment 3 Jakub Zytka 2014-07-23 09:57:54 UTC
Created attachment 920183 [details]
test case exposing problem

Comment 4 Jakub Zytka 2014-07-23 09:58:47 UTC
Created attachment 920184 [details]
generated assembly

Comment 8 Marek Polacek 2014-07-28 07:42:44 UTC
On 4.4 branch this started with http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=148601 - part of PR40389 fix (4.5 and newer are fine).  There doesn't seem to be anything that we could backport, unfortunately.

What happens here is that the code sinking moves the statement that is setting up the vtable pointer:

+Sinking # t_8 = VDEF <t_7>
+t._vptr.T = &_ZTV1T[2];
+ from bb 2 to bb 4
 int main() ()
   int pretmp.74;
@@ -444,7 +98,6 @@
 <bb 2>:
   t = empty (); [return slot optimization]
-  t._vptr.T = &_ZTV1T[2];
 <bb 3>:
   # i_15 = PHI <i_4(7), 0(2)>
@@ -459,6 +112,7 @@
   goto <bb 3>;
 <bb 4>:
+  t._vptr.T = &_ZTV1T[2];
   __asm__ __volatile__("" : "memory");
   D.2196_1 = a.data[0];
   if (D.2196_1 != 0)

The workaround is either to use -fno-tree-sink, or to add a memory barrier 
asm volatile ("" ::: "memory"); before Array aa; line in the original reproducer.

Comment 9 Jakub Zytka 2014-07-28 08:13:39 UTC
Thank you very much. I suppose -fno-tree-sink will do. 
We'll check it for performance, but I guess the impact will be negligible.

Comment 10 Matt Newsome 2014-07-28 08:34:50 UTC
(In reply to Jakub Zytka from comment #9)
> Thank you very much. I suppose -fno-tree-sink will do. 
> We'll check it for performance, but I guess the impact will be negligible.

Thanks for your understanding, Jakub. We do strive to provide fixes in Red Hat Enterprise Linux gcc wherever possible, but sometimes - as in this case where a patch would be relatively invasive into the code base - we need to favour overall stability.

I'm closing this bug out for now, but if you have a follow up query, please feel free to re-open this bug or open a new one and we'll take a look.

Note You need to log in before you can comment on or make changes to this bug.