diff mbox

Fix a C++ crash with may_alias type attribute

Message ID AM4PR0701MB2162496C5F59CCB971225B63E40B0@AM4PR0701MB2162.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger April 4, 2017, 9:29 p.m. UTC 
On 04/04/17 14:58, Jonathan Wakely wrote:
> On 04/04/17 12:39 +0000, Bernd Edlinger wrote:
>> Hi,
>>
>> I noticed that the already created reference and pointer types
>> are left in an inconsistent state if the may_alias attribute
>> is added to a class, in some cases.
>>
>> The attached patch fixes this by adding another loop over
>> all type variants of each pointer and reference type.
>>
>> The new test case 20_util/any/assign/2a.cc is just a
>> clone of 20_util/any/assign/2.cc with the may_alias
>> attribute at the right place.
>
> The new test doesn't really belong in the libstdc++, does it?
>
> If this is a bug in the front-end then a reduced version of the test
> belongs in the g++ testsuite.
>

Yes, I moved the test case to the g++.dg/lto branch, where it
fortunately also works (I forgot to mention that the crash only
happened with -flto).

Is it Ok for trunk?


Thanks
Bernd.

Comments

Ville Voutilainen April 4, 2017, 10:33 p.m. UTC | #1 
On 5 April 2017 at 00:29, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>> The new test doesn't really belong in the libstdc++, does it?
>>
>> If this is a bug in the front-end then a reduced version of the test
>> belongs in the g++ testsuite.
>>
>
> Yes, I moved the test case to the g++.dg/lto branch, where it
> fortunately also works (I forgot to mention that the crash only
> happened with -flto).
>
> Is it Ok for trunk?


That's not a reduced test. You need to remove dependencies to library
facilities.
Bernd Edlinger April 5, 2017, 2:34 a.m. UTC | #2 
On 04/05/17 00:33, Ville Voutilainen wrote:
> On 5 April 2017 at 00:29, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:

>>> The new test doesn't really belong in the libstdc++, does it?

>>>

>>> If this is a bug in the front-end then a reduced version of the test

>>> belongs in the g++ testsuite.

>>>

>>

>> Yes, I moved the test case to the g++.dg/lto branch, where it

>> fortunately also works (I forgot to mention that the crash only

>> happened with -flto).

>>

>> Is it Ok for trunk?

>

>

> That's not a reduced test. You need to remove dependencies to library

> facilities.

>


I replaced the dependency to VERIFY and used a own macro,
and verified, that the test still produce the error.

It is IMHO not worth to invest more time than this in a test
for an already obvious bug-fix.


Bernd.
Jason Merrill April 17, 2017, 8:04 p.m. UTC | #3 
OK.

On Tue, Apr 4, 2017 at 10:34 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 04/05/17 00:33, Ville Voutilainen wrote:
>> On 5 April 2017 at 00:29, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>>>> The new test doesn't really belong in the libstdc++, does it?
>>>>
>>>> If this is a bug in the front-end then a reduced version of the test
>>>> belongs in the g++ testsuite.
>>>>
>>>
>>> Yes, I moved the test case to the g++.dg/lto branch, where it
>>> fortunately also works (I forgot to mention that the crash only
>>> happened with -flto).
>>>
>>> Is it Ok for trunk?
>>
>>
>> That's not a reduced test. You need to remove dependencies to library
>> facilities.
>>
>
> I replaced the dependency to VERIFY and used a own macro,
> and verified, that the test still produce the error.
>
> It is IMHO not worth to invest more time than this in a test
> for an already obvious bug-fix.
>
>
> Bernd.
diff mbox

Patch

gcc/cp
2017-04-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR c++/80287
        * class.c (fixup_may_alias): Fix all type variants.

gcc/testsuite
2017-04-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR c++/80287
        * g++.dg/lto/pr80287_0.C: New test.


Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 246605)
+++ gcc/cp/class.c	(working copy)
@@ -2060,12 +2060,14 @@  fixup_type_variants (tree t)
 static void
 fixup_may_alias (tree klass)
 {
-  tree t;
+  tree t, v;
 
   for (t = TYPE_POINTER_TO (klass); t; t = TYPE_NEXT_PTR_TO (t))
-    TYPE_REF_CAN_ALIAS_ALL (t) = true;
+    for (v = TYPE_MAIN_VARIANT (t); v; v = TYPE_NEXT_VARIANT (v))
+      TYPE_REF_CAN_ALIAS_ALL (v) = true;
   for (t = TYPE_REFERENCE_TO (klass); t; t = TYPE_NEXT_REF_TO (t))
-    TYPE_REF_CAN_ALIAS_ALL (t) = true;
+    for (v = TYPE_MAIN_VARIANT (t); v; v = TYPE_NEXT_VARIANT (v))
+      TYPE_REF_CAN_ALIAS_ALL (v) = true;
 }
 
 /* Early variant fixups: we apply attributes at the beginning of the class
Index: gcc/testsuite/g++.dg/lto/pr80287_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr80287_0.C	(revision 0)
+++ gcc/testsuite/g++.dg/lto/pr80287_0.C	(working copy)
@@ -0,0 +1,92 @@ 
+// { dg-lto-options { "-std=gnu++17" } }
+// { dg-lto-do run }
+
+// Copyright (C) 2014-2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <any>
+#define VERIFY(x) if (!(x)) __builtin_abort()
+
+using std::any;
+using std::any_cast;
+
+bool moved = false;
+bool copied = false;
+
+struct X
+{
+  X() = default;
+  X(const X&) { copied = true; }
+  X(X&& x) { moved = true; }
+};
+
+struct X2
+{
+  X2() = default;
+  X2(const X2&) { copied = true; }
+  X2(X2&& x) noexcept { moved = true; }
+}__attribute((may_alias));
+
+void test01()
+{
+  moved = false;
+  X x;
+  any a1;
+  a1 = x;
+  VERIFY(moved == false);
+  any a2;
+  copied = false;
+  a2 = std::move(x);
+  VERIFY(moved == true);
+  VERIFY(copied == false);
+}
+
+void test02()
+{
+  moved = false;
+  X x;
+  any a1;
+  a1 = x;
+  VERIFY(moved == false);
+  any a2;
+  copied = false;
+  a2 = std::move(a1);
+  VERIFY(moved == false);
+  VERIFY(copied == false);
+}
+
+void test03()
+{
+  moved = false;
+  X2 x;
+  any a1;
+  a1 = x;
+  VERIFY(copied && moved);
+  any a2;
+  moved = false;
+  copied = false;
+  a2 = std::move(a1);
+  VERIFY(moved == true);
+  VERIFY(copied == false);
+}
+
+int main()
+{
+  test01();
+  test02();
+  test03();
+}