Re: [PATCHES, PING*5] Enhance standard DWARF for Ada

[Pierre-Marie de Rodat <derodat at adacore dot com>]

On 02/25/2016 10:48 AM, Jakub Jelinek wrote:
> The first one just fixes what I mainly care about, the committed patch
> assumed that DW_TAG_dwarf_procedure is always only created for the Ada
> variable sized structures or whatever it was meant for, which is not the
> case, and thus if we emit DW_TAG_dwarf_procedure for some other reason,
> it would be pruned as unused even when it is actually used (and result in
> a DIE reference to the compilation unit header, which is always invalid).

This first patch looks good to me, as a good enough and simple fix.

> But, looking at the points where you use DW_TAG_dwarf_procedure for the Ada
> things, I can't see how it can actually work at all, though there is no
> testsuite coverage, so it is hard to find out for real.
> The thing is, current code sets die_perennial_p on type DIEs and their
> parents, but nothing else.  In particular, type DIEs are identified by
> being returned from lookup_type_die, thus earlier passed to
> equate_type_number_to_die.  I don't see that this would ever be the case
> of DW_TAG_dwarf_procedure though, I see the return of
> function_to_dwarf_procedure being used as dw_loc_oprnd1.v.val_die_ref.die
> of a DW_OP_call4 that is somewhere used in some location description that is
> perhaps used somewhere in some type DIE computation.

I introduced a DW_OP_call* traversal for this:

  * prune_unused_types_mark traverses attributes using
    prune_unused_types_walk_attribs;

  * â_walk_attribs walks location descriptions and location lists using
    â_walk_loc_descr

  * â_walk_loc_descr marks DWARF procedures referenced by DW_OP_call*
    operations.

So all DWARF procedures referenced this way are not supposed to be 
pruned (I checked: no problem for the Ada types I tested). As you 
noticed, I did not realize that there were other DWARF procedure 
producers, hence the assumption that this was enought to mark all DWARF 
procs.

> So IMHO the second patch
> makes more sense, and if you (for GCC 7?) want to prune really unused
> DW_TAG_dwarf_procedure, you need to add code that will really walk all of
> the debuginfo, rather than just type DIEs themselves, and look if location
> descriptions (in .debug_info or .debug_loc) reference those
> DW_TAG_dwarf_procedure and mark the DW_TAG_dwarf_procedure.

I just had a look: the prune_unused_type_mark pass already reaches the 
DW_OP_GNU_implicit_pointer operation, itâs just that 
prune_unused_types_walk_loc_descr does not know about this kind of 
operation. I think the attached patch is a more general fix for that. 
What do you think?

(bootstrapped and regtested on x86_64-linux)

--
Pierre-Marie de Rodat

>From 671199e8a7da326e54e081b5c368f93428455e98 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Thu, 25 Feb 2016 11:37:22 +0100
Subject: [PATCH] DWARF: fix debug info for implicit pointers to string
 constants

2016-02-25  Pierre-Marie de Rodat  <derodat@adacore.com>

        PR debug/69947
        * dwarf2out.c (prune_unused_types_walk_loc_descr): Recurse on
          all dw_val_class_die_ref operands, not just the ones for
          DW_OP_call* operations.

2016-02-25  Jakub Jelinek  <jakub@redhat.com>

        PR debug/69947
        * gcc.dg/guality/pr69947.c: New test.
---
 gcc/dwarf2out.c                        | 16 +++++++---------
 gcc/testsuite/gcc.dg/guality/pr69947.c | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/guality/pr69947.c

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 97e192b..37ccd3a 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -25639,16 +25639,14 @@ static void
 prune_unused_types_walk_loc_descr (dw_loc_descr_ref loc)
 {
   for (; loc != NULL; loc = loc->dw_loc_next)
-    switch (loc->dw_loc_opc)
-      {
-      case DW_OP_call2:
-      case DW_OP_call4:
-      case DW_OP_call_ref:
+    {
+      if (loc->dw_loc_oprnd1.val_class == dw_val_class_die_ref
+	  && !loc->dw_loc_oprnd1.v.val_die_ref.external)
 	prune_unused_types_mark (loc->dw_loc_oprnd1.v.val_die_ref.die, 1);
-	break;
-      default:
-	break;
-      }
+      if (loc->dw_loc_oprnd2.val_class == dw_val_class_die_ref
+	  && !loc->dw_loc_oprnd2.v.val_die_ref.external)
+	prune_unused_types_mark (loc->dw_loc_oprnd2.v.val_die_ref.die, 1);
+    }
 }
 
 /* Given DIE that we're marking as used, find any other dies
diff --git a/gcc/testsuite/gcc.dg/guality/pr69947.c b/gcc/testsuite/gcc.dg/guality/pr69947.c
new file mode 100644
index 0000000..6280ed5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/pr69947.c
@@ -0,0 +1,22 @@
+/* PR debug/69947 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+#include "../nop.h"
+
+static const char *c = "foobar";
+
+__attribute__((noinline, noclone)) void
+foo (void)
+{
+  static const char a[] = "abcdefg";
+  const char *b = a;		/* { dg-final { gdb-test 14 "c\[2\]" "'o'" } } */
+  asm (NOP : : : "memory");	/* { dg-final { gdb-test 14 "b\[4\]" "'e'" } } */
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
-- 
2.3.3.199.g52cae64