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> >).
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; }
I think the real culprit is r228679. Honza?
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