diff mbox

[PR78112] Remove the g++.dg/pr78112.C testcase

Message ID c0c66c41-7b18-d1ac-2fc0-361b79aa63ba@adacore.com
State New
Headers show

Commit Message

Pierre-Marie de Rodat Dec. 8, 2016, 11:04 a.m. UTC 
Hello Mike,

On 11/30/2016 09:12 PM, Mike Stump wrote:
> So, I noticed this and didn't see who you wanted to review it so,

> since it was C++, I thought I'd take a look at it.  Ick.  Complex

> issue.


I did not have anyone special in mind actually, so thank you for having 
had a look. :-)

> One way to test this would be to have a internal check in the

> compiler for the thing you don't want to happen as an assert, and

> then have the unpatched compiler abort when given the test case

> before the work that cause fixed the original PR.  The test case then

> shows the failure, and should anyone break it, the internal check

> will catch the problem and abort, thus causing the test case to then

> fail (again).


I guess the internal check of relevance here would be “check that when 
adding an attribute to a DIE, existing attributes on this DIE don’t have 
the same DW_AT_* kind”. Unfortunately, we know this invariant is still 
violated (and was before my first changes affecting this), so I don’t 
think this is possible right now unless someone spends more time 
investigating why there are duplicate attributes and how to fix them.

> Then, you only need to compile the test case and expect a non-zero

> output from the compilation.  On darwin, the excess message from

> dsymutil should be findable by dejagnu and should also be able to

> fail the test case on darwin.


That being said, I’m still wondering why dsymutil produces no error even 
though there are still duplicate attributes (less of them, but still 
some). Maybe because DW_AT_object_pointer is not emitted on Darwin 
(which uses -gdwarf-2 by default) and because the DW_AT_inline duplicate 
does not appear there as well? Anyway…

> If you like that design (and a dwarf maintainer likes that design),

> then you can fix this test case by removing:

>

>> -/* { dg-final { scan-assembler-times DW_AT_inline 6 { xfail *-*-aix* } } } */

>> -/* { dg-final { scan-assembler-times DW_AT_object_pointer 37 { xfail *-*-aix* } } } */

>

> alone and otherwise keep the full test case.


I’m fine with keeping this testcase and updating it as you said. It will 
be useful only for Darwin users, though, but I guess that’s better than 
nothing as it will clearly relate the regression to this whole 
discussion when there is a failure on this platform.

> When looking at the test case, I wonder just how reduced it really

> was.  The last possible option would be to reduce the test case

> further and see if the problem can be eliminated that way.  Again,

> I'll pre-approve the test suite part of any of those solutions you

> like best.


I already tried to reduce it as much as possible: see 
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78112#c9>. On my 
x86_64-linux box, I tried to build several cross-compilers 
(x86_64-apple-darwin16.3.0 and arm-none-eabi) in order to see how this 
reduced reproducer behaves: it looks like we have a consistent number of 
DW_AT_object_pointer attributes for it as long as we force -gdwarf-4 
(the default is -gdwarf-2 on Darwin), which is: 18 times. So here is an 
updated patch that removes the trouble some check in pr78112.C and that 
adds the reduced testcase as pr78112-2.C.

Thank you for the pre-approval: I’ve just pushed this fix. For the 
record, I’ve checked it runs fine on x86_64-linux and 
x86_64-apple-darwin-16.3.0 and I’ve checked manually we have the correct 
number of attributes with a partial arm-none-abi compiler (i.e. just 
cc1plus).

-- 
Pierre-Marie de Rodat

Comments

Mike Stump Dec. 8, 2016, 5:08 p.m. UTC | #1 
On Dec 8, 2016, at 3:04 AM, Pierre-Marie de Rodat <derodat@adacore.com> wrote:
> Thank you for the pre-approval: I’ve just pushed this fix. For the record, I’ve checked it runs fine on x86_64-linux and x86_64-apple-darwin-16.3.0 and I’ve checked manually we have the correct number of attributes with a partial arm-none-abi compiler (i.e. just cc1plus).


Thanks.
diff mbox

Patch

From fa8d7ca05a8711e261b5c4cfeec885c3ecede508 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Wed, 30 Nov 2016 13:57:34 +0100
Subject: [PATCH] [PR78112] Remove platform-dependent checks in
 g++.dg/pr78112.C

... as there checks failed on many platforms. As a replacement, this
commit also adds a new testcase from source reduction. The hope is that
this new testcase will get a consistent output across all platforms.

gcc/testsuite/
	PR debug/78112
	* g++.dg/pr78112.C: Remove platform-dependent checks.
	* g++.dg/pr78112-2.C: New testcase.
---
 gcc/testsuite/g++.dg/pr78112-2.C | 13 +++++++++++++
 gcc/testsuite/g++.dg/pr78112.C   |  2 --
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr78112-2.C

diff --git a/gcc/testsuite/g++.dg/pr78112-2.C b/gcc/testsuite/g++.dg/pr78112-2.C
new file mode 100644
index 0000000..d9d18ff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr78112-2.C
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-g -dA -gdwarf-4 -std=gnu++11" } */
+/* { dg-options "-g -dA -std=gnu++11 -gdwarf-4" } */
+/* { dg-final { scan-assembler-times DW_AT_object_pointer 18 } } */
+
+void run (int *int_p, void(*func)(int *)) { func (int_p); }
+namespace foo {
+   struct Foo {
+      int a;
+      Foo() { run (&a, [](int *int_p) { *int_p = 0; }); }
+   };
+}
+int main (void) { foo::Foo f; }
diff --git a/gcc/testsuite/g++.dg/pr78112.C b/gcc/testsuite/g++.dg/pr78112.C
index 986171d..8312292 100644
--- a/gcc/testsuite/g++.dg/pr78112.C
+++ b/gcc/testsuite/g++.dg/pr78112.C
@@ -1,7 +1,5 @@ 
 /* { dg-do compile } */
 /* { dg-options "-g -dA -std=gnu++11" } */
-/* { dg-final { scan-assembler-times DW_AT_inline 6 { xfail *-*-aix* } } } */
-/* { dg-final { scan-assembler-times DW_AT_object_pointer 37 { xfail *-*-aix* } } } */
 namespace std
 {
 template <typename _Tp> struct integral_constant
-- 
2.10.2