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
>From fa8d7ca05a8711e261b5c4cfeec885c3ecede508 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <dero...@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
