| Summary: | [6 Regression] Optimizations O1/O2 makes std::array value incorrect when passed to function | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Simon Giraudot <gee> |
| Component: | middle-end | Assignee: | Richard Biener <rguenth> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | hubicka, Laurent.Rineau__gcc |
| Priority: | P1 | Keywords: | wrong-code |
| Version: | 6.0 | ||
| Target Milestone: | 6.0 | ||
| Host: | Target: | ||
| Build: | Known to work: | 5.3.0 | |
| Known to fail: | 6.0 | Last reconfirmed: | 2016-01-29 00:00:00 |
| Attachments: | Source code generating the bug | ||
This is not a libstdc++ issue. gcc-5 and clang don't miscompile the example. Started with r229265: commit 2425f3b6ce0532030e1e4274d0fc82a5d7e16c79 Author: hubicka <hubicka@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri Oct 23 18:05:55 2015 +0000 * fold-const.c (operand_equal_p): Do not compare TYPE_MODE when comparing addresses. foo (_13, _11); _9 = &MEM[(const int[2] &)t_2(D) + 8][1]; foo (_11, _9); [tail call] Trying to reduce the issue. (In reply to Markus Trippelsdorf from comment #2) > Started with r229265: > > commit 2425f3b6ce0532030e1e4274d0fc82a5d7e16c79 > Author: hubicka <hubicka@138bc75d-0d04-0410-961f-82ee72b054a4> > Date: Fri Oct 23 18:05:55 2015 +0000 > > * fold-const.c (operand_equal_p): Do not compare TYPE_MODE when > comparing addresses. I think this just exposed the issue somewhere else. We go from:
_3 = &MEM[(const struct array[2] &)t_2(D)][0];
test1 (_3);
_5 = &MEM[(const struct array[2] &)t_2(D)][1];
test1 (_5);
_7 = &MEM[(const int[2] &)t_2(D)][1];
foo (_3, _7);
_9 = &MEM[(const int[2] &)t_2(D) + 8][1];
foo (_5, _9);
To:
_3 = &MEM[(const struct array[2] &)t_2(D)][0];
test1 (_3);
_5 = &MEM[(const struct array[2] &)t_2(D)][1];
test1 (_5);
foo (_3, _5);
_9 = &MEM[(const int[2] &)t_2(D) + 8][1];
foo (_5, _9); [tail call]
I have been trying to remove std::array but I can't.
Here is my current testcase:
#include <array>
typedef int type;
using std::array;
typedef array<array<type, 2>, 2> Matrix;
int t = 0;
void foo(const type &px, const type &py) __attribute__((noinline,noclone));
void foo(const type &px, const type &py)
{
if ((t&1)==0)
{
if (px != 1)
__builtin_abort ();
if (py != 2)
__builtin_abort ();
} else if (t&1)
{
if (px != 3)
__builtin_abort ();
if (py != 4)
__builtin_abort ();
}
t++;
}
void test1 (const array<type, 2>& p) __attribute__((noinline, noclone));
void test1 (const array<type, 2>& p)
{
foo(p[0], p[1]);
}
void test (const Matrix& t) __attribute__((noinline, noclone));
void test (const Matrix& t)
{
test1 (t[0]);
test1 (t[1]);
foo(t[0][0], t[0][1]);
foo(t[1][0], t[1][1]);
}
int main (int, char**)
{
Matrix t;
t[0][0] = 1;
t[0][1] = 2;
t[1][0] = 3;
t[1][1] = 4;
test (t);
return 0;
}
markus@x4 tmp % cat array.ii
template <typename _Tp, long _Nm> struct A {
typedef _Tp _Type[_Nm];
static _Tp &_S_ref(const _Type &p1, int p2) {
return const_cast<_Tp &>(p1[p2]);
}
};
template <typename _Tp, long _Nm> struct B {
typedef A<_Tp, _Nm> _AT_Type;
typename _AT_Type::_Type _M_elems;
_Tp &operator[](long p1) const { return _AT_Type::_S_ref(_M_elems, p1); }
};
int t;
void foo(int p1, int &p2) {
if ((t & 1) == 0) {
if (p1 != 1)
__builtin_abort();
if (p2 != 2)
__builtin_abort();
}
t++;
}
__attribute__((noinline))
void test1(const B<int, 2> &p1) { foo(p1[0], p1[1]); }
void test(B<B<int, 2>, 2> &p1) {
test1(p1[0]);
test1(p1[1]);
foo(p1[0][0], p1[0][1]);
}
int main() {
B<B<int, 2>, 2> t;
t[0][0] = 1;
t[0][1] = 2;
test(t);
}
Let me have a look. So the issue is that
&MEM[(const struct B[2] &)p1_2(D)][1]
and
&MEM[(const int[2] &)p1_2(D)][1]
are considered equal even though we have int[] vs. B[] and thus different
element sizes and different offsets (but same array indices) here.
For COMPONENT_REFs we compare the FIELD_DECL but for ARRAY_REFs and also
for IMAGPART_EXPR we fail to consider the element size.
Not that comparing TYPE_PRECISION on MEM_REFs of ARRAY_TYPE makes any sense,
or TYPE_UNSIGNED.
So the old code had bugs as well here (consider array types with different
low bound).
Index: fold-const.c
===================================================================
--- fold-const.c (revision 233447)
+++ fold-const.c (working copy)
@@ -3008,8 +3008,15 @@ operand_equal_p (const_tree arg0, const_
flags &= ~OEP_ADDRESS_OF;
return OP_SAME (0);
- case REALPART_EXPR:
case IMAGPART_EXPR:
+ /* Require the same offset. */
+ if (!operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
+ TYPE_SIZE (TREE_TYPE (arg1)),
+ flags & ~OEP_ADDRESS_OF)))
+ return 0;
+
+ /* Fallthru. */
+ case REALPART_EXPR:
case VIEW_CONVERT_EXPR:
return OP_SAME (0);
@@ -3049,17 +3056,24 @@ operand_equal_p (const_tree arg0, const_
case ARRAY_REF:
case ARRAY_RANGE_REF:
- /* Operands 2 and 3 may be null.
- Compare the array index by value if it is constant first as we
- may have different types but same value here. */
if (!OP_SAME (0))
return 0;
flags &= ~OEP_ADDRESS_OF;
+ /* Compare the array index by value if it is constant first as we
+ may have different types but same value here.
+ Compare low bound and element size as with OEP_ADDRESS_OF
+ we have to account for the offset of the ref. */
return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
TREE_OPERAND (arg1, 1))
|| OP_SAME (1))
- && OP_SAME_WITH_NULL (2)
- && OP_SAME_WITH_NULL (3));
+ && operand_equal_p (array_ref_low_bound
+ (CONST_CAST_TREE (arg0)),
+ array_ref_low_bound
+ (CONST_CAST_TREE (arg1)), flags)
+ && operand_equal_p (array_ref_element_size
+ (CONST_CAST_TREE (arg0)),
+ array_ref_element_size
+ (CONST_CAST_TREE (arg1)), flags));
case COMPONENT_REF:
/* Handle operand 2 the same as for ARRAY_REF. Operand 0
Exposes a similar issue in stmt_kills_ref_p which does 2248 /* Just compare the outermost handled component, if 2249 they are equal we have found a possible common 2250 base. */ 2251 tree saved_base0 = TREE_OPERAND (base, 0); 2252 TREE_OPERAND (base, 0) = integer_zero_node; 2253 bool res = operand_equal_p (lhs, base, 0); 2254 TREE_OPERAND (base, 0) = saved_base0; 2255 if (res) 2256 break; and thus also happily will treat unrelated ARRAY_REFs (different element size or low bound) as equal ... Fixed. Author: rguenth Date: Thu Feb 18 14:34:59 2016 New Revision: 233520 URL: https://gcc.gnu.org/viewcvs?rev=233520&root=gcc&view=rev Log: 2016-02-18 Richard Biener <rguenther@suse.de> PR middle-end/69553 * fold-const.c (operand_equal_p): Properly compare offsets for IMAGPART_EXPR and ARRAY_REF. * g++.dg/torture/pr69553.C: New testcase. Added: trunk/gcc/testsuite/g++.dg/torture/pr69553.C Modified: trunk/gcc/ChangeLog trunk/gcc/fold-const.c trunk/gcc/testsuite/ChangeLog |
Created attachment 37521 [details] Source code generating the bug The following example displays incorrect values when compiling with optimizations -O1 or -O2 (but not without optimization and not with -O3): ---------------------- #include <iostream> #include <array> typedef std::array<std::array<double, 2>, 2> Matrix; void foo(const double &px, const double &py) { std::cerr << px << " " << py << std::endl; } std::ostream& operator<<(std::ostream& os, const std::array<double, 2>& p) { return os << p[0] << " " << p[1]; } void test (const Matrix& t) { std::cerr << "In test: " << std::endl << t[0] << std::endl << t[1] << std::endl; std::cerr << "In foo: " << std::endl; foo(t[0][0], t[0][1]); foo(t[1][0], t[1][1]); } int main (int, char**) { Matrix t = {{ {{1,2}}, {{3,4}} }}; test (t); return 0; } ---------------------- (Compiled with 'g++-6 -std=c++11 -O1 test_bug_array.cpp') Without optimization (or with O3), i have the following output: In test: 1 2 3 4 In foo: 1 2 3 4 Which makes sense. Now if I turn on O1 or O2, I get: In test: 1 2 3 4 In foo: 1 3 3 4 The bug does not appear when using boost::array or when replacing the structure by a simple C-array (double[2][2]) or by pairs (std::pair<std::pair<double> >).