public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
@ 2016-07-23 15:25 Alexandre Oliva
  2016-08-02  3:08 ` Alexandre Oliva
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Alexandre Oliva @ 2016-07-23 15:25 UTC (permalink / raw)
  To: gcc-patches

We used to emit, in debug information, the values bound to pointer to
member function template parameters only when they were NULL or
virtual member functions, because those can be represented with
DW_AT_const_value.

In order to represent the symbolic pointer to member function
constants for non-virtual member functions, we'd need to be able to
emit relocations for part of DW_AT_const_value, which we don't.  The
more viable alternative is to use DW_AT_location to represent such
values, as slated for inclusion in DWARFv5, according to
<URL:http://www.dwarfstd.org/ShowIssue.php?issue=130412.1>.

With this patch, when we can't emit a DW_AT_const_value, we emit each
"member" of the pointer to member function "record" as a
DW_OP_stack_value DW_OP_piece, as long as the referenced member
function is output in the same translation unit, otherwise we'd get
relocations to external symbols, something to avoid in debug sections.

Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu, and manually
cross-tested with both endiannesses (just to be sure ;-) with a
mips64-elf target.  Ok to install?

for  gcc/ChangeLog

	PR debug/49366
	* dwarf2out.c (loc_list_from_tree_1): Expand some CONSTRUCTORs
	in DW_OP_pieces, just enough to handle pointers to member
	functions.
	(gen_remaining_tmpl_value_param_die_attribute): Use a location
	expression on DWARFv5 if a constant value doesn't work.

for  gcc/testsuite/ChangeLog

	PR debug/49366
	* g++.dg/debug/dwarf2/template-params-12.H: New.
	* g++.dg/debug/dwarf2/template-params-12f.C: New.
	* g++.dg/debug/dwarf2/template-params-12g.C: New.
	* g++.dg/debug/dwarf2/template-params-12n.C: New.
	* g++.dg/debug/dwarf2/template-params-12s.C: New.
	* g++.dg/debug/dwarf2/template-params-12u.C: New.
	* g++.dg/debug/dwarf2/template-params-12v.C: New.
	* g++.dg/debug/dwarf2/template-params-12w.C: New.
---
 gcc/dwarf2out.c                                    | 93 +++++++++++++++++++++-
 .../g++.dg/debug/dwarf2/template-params-12.H       | 17 ++++
 .../g++.dg/debug/dwarf2/template-params-12f.C      |  7 ++
 .../g++.dg/debug/dwarf2/template-params-12g.C      |  7 ++
 .../g++.dg/debug/dwarf2/template-params-12n.C      | 10 +++
 .../g++.dg/debug/dwarf2/template-params-12s.C      |  8 ++
 .../g++.dg/debug/dwarf2/template-params-12u.C      |  7 ++
 .../g++.dg/debug/dwarf2/template-params-12v.C      |  6 ++
 .../g++.dg/debug/dwarf2/template-params-12w.C      |  6 ++
 9 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index e3cb586..c658220 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -16142,6 +16142,89 @@ loc_list_from_tree_1 (tree loc, int want_address,
     case COMPLEX_CST:
       if ((ret = cst_pool_loc_descr (loc)))
 	have_address = 1;
+      else if (TREE_CODE (loc) == CONSTRUCTOR)
+	{
+	  tree type = TREE_TYPE (loc);
+	  unsigned HOST_WIDE_INT size = int_size_in_bytes (type);
+	  unsigned HOST_WIDE_INT offset = 0;
+	  unsigned HOST_WIDE_INT cnt;
+	  constructor_elt *ce;
+
+	  if (TREE_CODE (type) == RECORD_TYPE)
+	    {
+	      /* This is very limited, but it's enough to output
+		 pointers to member functions, as long as the
+		 referenced function is defined in the current
+		 translation unit.  */
+	      FOR_EACH_VEC_SAFE_ELT (CONSTRUCTOR_ELTS (loc), cnt, ce)
+		{
+		  tree val = ce->value;
+
+		  tree field = ce->index;
+
+		  if (val)
+		    STRIP_NOPS (val);
+
+		  if (!field || DECL_BIT_FIELD (field))
+		    {
+		      expansion_failed (loc, NULL_RTX,
+					"bitfield in record type constructor");
+		      size = offset = (unsigned HOST_WIDE_INT)-1;
+		      ret = NULL;
+		      break;
+		    }
+
+		  HOST_WIDE_INT fieldsize = tree_to_shwi (DECL_SIZE_UNIT (field));
+		  unsigned HOST_WIDE_INT pos = int_byte_position (field);
+		  gcc_assert (pos + fieldsize <= size);
+		  if (pos < offset)
+		    {
+		      expansion_failed (loc, NULL_RTX,
+					"out-of-order fields in record constructor");
+		      size = offset = (unsigned HOST_WIDE_INT)-1;
+		      ret = NULL;
+		      break;
+		    }
+		  if (pos > offset)
+		    {
+		      ret1 = new_loc_descr (DW_OP_piece, pos - offset, 0);
+		      add_loc_descr (&ret, ret1);
+		      offset = pos;
+		    }
+		  if (val && fieldsize != 0)
+		    {
+		      ret1 = loc_descriptor_from_tree (val, want_address, context);
+		      if (!ret1)
+			{
+			  expansion_failed (loc, NULL_RTX,
+					    "unsupported expression in field");
+			  size = offset = (unsigned HOST_WIDE_INT)-1;
+			  ret = NULL;
+			  break;
+			}
+		      add_loc_descr (&ret, ret1);
+		    }
+		  if (fieldsize)
+		    {
+		      ret1 = new_loc_descr (DW_OP_piece, fieldsize, 0);
+		      add_loc_descr (&ret, ret1);
+		      offset = pos + fieldsize;
+		    }
+		}
+
+	      if (offset != size)
+		{
+		  ret1 = new_loc_descr (DW_OP_piece, size - offset, 0);
+		  add_loc_descr (&ret, ret1);
+		  offset = size;
+		}
+
+	      have_address = !!want_address;
+	    }
+	  else
+	    expansion_failed (loc, NULL_RTX,
+			      "constructor of non-record type");
+	}
       else
       /* We can construct small constants here using int_loc_descriptor.  */
 	expansion_failed (loc, NULL_RTX,
@@ -24177,7 +24260,15 @@ gen_remaining_tmpl_value_param_die_attribute (void)
       FOR_EACH_VEC_ELT (*tmpl_value_parm_die_table, i, e)
 	{
 	  if (!tree_add_const_value_attribute (e->die, e->arg))
-	    (*tmpl_value_parm_die_table)[j++] = *e;
+	    {
+	      dw_loc_descr_ref loc = NULL;
+	      if (dwarf_version >= 5 || !dwarf_strict)
+		loc = loc_descriptor_from_tree (e->arg, 2, NULL);
+	      if (loc)
+		add_AT_loc (e->die, DW_AT_location, loc);
+	      else
+		(*tmpl_value_parm_die_table)[j++] = *e;
+	    }
 	}
       tmpl_value_parm_die_table->truncate (j);
     }
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
new file mode 100644
index 0000000..24c26e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
@@ -0,0 +1,17 @@
+// Tests for the fix for PR debug/49366.
+struct B {
+  void g();
+  virtual void v() = 0;
+  virtual void w();
+};
+void B::g() {}
+void B::w() {}
+struct S : B {
+  void f();
+  void v();
+  void u();
+};
+void S::f() {}
+void S::v() {}
+template <void (B::*MF)()> void t() {}
+template <void (S::*MF)()> void t() {}
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
new file mode 100644
index 0000000..e2da829
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
@@ -0,0 +1,7 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]*\[^\n\]* DW_AT_location\n\[^\n\]* DW_OP_addr\n\[^\n\]*_ZN1S1fEv\[^\n\]*\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece\n\[^\n\]*\n\[^\n\]* DW_OP_lit0\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece" 1 } }
+#include "template-params-12.H"
+/* We get a location list with a pair of DW_OP_pieces for pointers to
+   non-virtual member functions.  */
+template void t<&S::f>();
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
new file mode 100644
index 0000000..813f71d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
@@ -0,0 +1,7 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]*\[^\n\]* DW_AT_location\n\[^\n\]* DW_OP_addr\n\[^\n\]*_ZN1B1gEv\[^\n\]*\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece\n\[^\n\]*\n\[^\n\]* DW_OP_lit0\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece" 1 } }
+#include "template-params-12.H"
+/* We get a location list with a pair of DW_OP_pieces for pointers to
+   non-virtual member functions.  */
+template void t<&S::g>();
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
new file mode 100644
index 0000000..d3c1f58
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
@@ -0,0 +1,10 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
+#include "template-params-12.H"
+/* We get const_value for NULL pointers to member functions.  */
+#if __cplusplus > 199711L // Ugh, C++98 barfs at both the cast and the overload.
+template void t<static_cast<void (S::*)()>(0)>();
+#else
+template void t<&S::v>();
+#endif
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
new file mode 100644
index 0000000..8940eaf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
@@ -0,0 +1,8 @@
+// { dg-options "-gdwarf-4 -gstrict-dwarf -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* \[^\n\]*DIE" 1 } }
+#include "template-params-12.H"
+/* We do NOT get a value or location for this one, because we've
+   enabled strict DWARF 4, and it could only be emitted as a location,
+   which is DWARF 5 only for template value params.  */
+template void t<&S::f>();
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
new file mode 100644
index 0000000..f79fa84
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
@@ -0,0 +1,7 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* \[^\n\]*DIE" 1 } }
+#include "template-params-12.H"
+/* We do NOT get a value or location for this one, because it would
+   require a relocation to an undefined symbol in a debug section.  */
+template void t<&S::u>();
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
new file mode 100644
index 0000000..11aabae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
@@ -0,0 +1,6 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
+#include "template-params-12.H"
+/* We get const_value for pointers to virtual member functions.  */
+template void t<&S::v>();
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C
new file mode 100644
index 0000000..11aabae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C
@@ -0,0 +1,6 @@
+// { dg-options "-gdwarf-2 -dA" }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
+#include "template-params-12.H"
+/* We get const_value for pointers to virtual member functions.  */
+template void t<&S::v>();
-- 
2.1.0


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-07-23 15:25 [PR49366] emit loc exprs for C++ non-virtual pmf template value parms Alexandre Oliva
@ 2016-08-02  3:08 ` Alexandre Oliva
  2016-08-05 17:31 ` Jason Merrill
  2016-08-24 11:19 ` Richard Biener
  2 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2016-08-02  3:08 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, ccoutant

On Jul 23, 2016, Alexandre Oliva <aoliva@redhat.com> wrote:

> We used to emit, in debug information, the values bound to pointer to
> member function template parameters only when they were NULL or
> virtual member functions, because those can be represented with
> DW_AT_const_value.

> In order to represent the symbolic pointer to member function
> constants for non-virtual member functions, we'd need to be able to
> emit relocations for part of DW_AT_const_value, which we don't.  The
> more viable alternative is to use DW_AT_location to represent such
> values, as slated for inclusion in DWARFv5, according to
> <URL:http://www.dwarfstd.org/ShowIssue.php?issue=130412.1>.

> With this patch, when we can't emit a DW_AT_const_value, we emit each
> "member" of the pointer to member function "record" as a
> DW_OP_stack_value DW_OP_piece, as long as the referenced member
> function is output in the same translation unit, otherwise we'd get
> relocations to external symbols, something to avoid in debug sections.

> Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu, and manually
> cross-tested with both endiannesses (just to be sure ;-) with a
> mips64-elf target.  Ok to install?

Ping?  (patch attached to gcc.gnu.org/PR49366)

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-07-23 15:25 [PR49366] emit loc exprs for C++ non-virtual pmf template value parms Alexandre Oliva
  2016-08-02  3:08 ` Alexandre Oliva
@ 2016-08-05 17:31 ` Jason Merrill
  2016-08-05 19:38   ` Alexandre Oliva
  2016-08-05 22:00   ` Alexandre Oliva
  2016-08-24 11:19 ` Richard Biener
  2 siblings, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2016-08-05 17:31 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches List

On Sat, Jul 23, 2016 at 10:30 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> We used to emit, in debug information, the values bound to pointer to
> member function template parameters only when they were NULL or
> virtual member functions, because those can be represented with
> DW_AT_const_value.
>
> In order to represent the symbolic pointer to member function
> constants for non-virtual member functions, we'd need to be able to
> emit relocations for part of DW_AT_const_value, which we don't. The
> more viable alternative is to use DW_AT_location to represent such
> values, as slated for inclusion in DWARFv5, according to
> <URL:http://www.dwarfstd.org/ShowIssue.php?issue=130412.1>.

> With this patch, when we can't emit a DW_AT_const_value, we emit each
> "member" of the pointer to member function "record" as a
> DW_OP_stack_value DW_OP_piece, as long as the referenced member
> function is output in the same translation unit, otherwise we'd get
> relocations to external symbols, something to avoid in debug sections.

I wonder if it would make sense to use weak references...

> +                 if (pos > offset)
> +                   {
> +                     ret1 = new_loc_descr (DW_OP_piece, pos - offset, 0);
> +                     add_loc_descr (&ret, ret1);
> +                     offset = pos;
> +                   }

This seems like it will emit a DW_OP_piece with nothing before it,
which I think is invalid DWARF.  Perhaps we should use DW_OP_lit0
first, or fail the expansion.

> +             if (offset != size)
> +               {
> +                 ret1 = new_loc_descr (DW_OP_piece, size - offset, 0);
> +                 add_loc_descr (&ret, ret1);
> +                 offset = size;
> +               }

Likewise.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-08-05 17:31 ` Jason Merrill
@ 2016-08-05 19:38   ` Alexandre Oliva
  2016-08-05 22:00   ` Alexandre Oliva
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2016-08-05 19:38 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches List

On Aug  5, 2016, Jason Merrill <jason@redhat.com> wrote:

> On Sat, Jul 23, 2016 at 10:30 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> With this patch, when we can't emit a DW_AT_const_value, we emit each
>> "member" of the pointer to member function "record" as a
>> DW_OP_stack_value DW_OP_piece, as long as the referenced member
>> function is output in the same translation unit, otherwise we'd get
>> relocations to external symbols, something to avoid in debug sections.

> I wonder if it would make sense to use weak references...

Hmm...  That certainly wouldn't avoid the relocation, but if the problem
is just the possibility that the symbol is undefined, that would work.
OTOH, it would hide the error one would expect out of a hard reference
in the same translation unit.  Could we ever have a weak reference in
debug info without a corresponding hard reference in the same
translation unit?  We'd have to take that into account.  I'll think
about it some more and maybe give that a shot.

>> +                 if (pos > offset)
>> +                   {
>> +                     ret1 = new_loc_descr (DW_OP_piece, pos - offset, 0);
>> +                     add_loc_descr (&ret, ret1);
>> +                     offset = pos;
>> +                   }

> This seems like it will emit a DW_OP_piece with nothing before it,
> which I think is invalid DWARF.

If so, there'd be a long-standing bug in the DW_OP examples in the DWARF
standard:

  DW_OP_reg0 DW_OP_piece 4 DW_OP_piece 4 DW_OP_fbreg -12 DW_OP_piece 4
    A twelve byte value whose first four bytes reside in register zero,
    whose middle four bytes are unavailable (perhaps due to
    optimization), and whose last four bytes are in memory, 12 bytes
    before the frame base.

However, this use seems consistent with the description of DW_OP_piece,
that says it takes an operand that describes the size of the preceding
simple location description.  An empty location description is-a simple
location description.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-08-05 17:31 ` Jason Merrill
  2016-08-05 19:38   ` Alexandre Oliva
@ 2016-08-05 22:00   ` Alexandre Oliva
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2016-08-05 22:00 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches List

On Aug  5, 2016, Jason Merrill <jason@redhat.com> wrote:

>> With this patch, when we can't emit a DW_AT_const_value, we emit each
>> "member" of the pointer to member function "record" as a
>> DW_OP_stack_value DW_OP_piece, as long as the referenced member
>> function is output in the same translation unit, otherwise we'd get
>> relocations to external symbols, something to avoid in debug sections.

> I wonder if it would make sense to use weak references...

I found the chunk of code that caused us to drop references to symbols
not defined in the current translation unit.  The comments at the end of
const_ok_for_output_1 don't sound not exactly hopeful:

  /* Avoid references to external symbols in debug info, on several targets
     the linker might even refuse to link when linking a shared library,
     and in many other cases the relocations for .debug_info/.debug_loc are
     dropped, so the address becomes zero anyway.  Hidden symbols, guaranteed
     to be defined within the same shared library or executable are fine.  */

Anyway, this change is something that could certainly be addressed as a
separate patch, since it would have effects over a lot more than just
pointers to member functions.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-07-23 15:25 [PR49366] emit loc exprs for C++ non-virtual pmf template value parms Alexandre Oliva
  2016-08-02  3:08 ` Alexandre Oliva
  2016-08-05 17:31 ` Jason Merrill
@ 2016-08-24 11:19 ` Richard Biener
  2016-08-30 23:15   ` Alexandre Oliva
  2 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2016-08-24 11:19 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: GCC Patches

On Sat, Jul 23, 2016 at 4:30 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> We used to emit, in debug information, the values bound to pointer to
> member function template parameters only when they were NULL or
> virtual member functions, because those can be represented with
> DW_AT_const_value.
>
> In order to represent the symbolic pointer to member function
> constants for non-virtual member functions, we'd need to be able to
> emit relocations for part of DW_AT_const_value, which we don't.  The
> more viable alternative is to use DW_AT_location to represent such
> values, as slated for inclusion in DWARFv5, according to
> <URL:http://www.dwarfstd.org/ShowIssue.php?issue=130412.1>.
>
> With this patch, when we can't emit a DW_AT_const_value, we emit each
> "member" of the pointer to member function "record" as a
> DW_OP_stack_value DW_OP_piece, as long as the referenced member
> function is output in the same translation unit, otherwise we'd get
> relocations to external symbols, something to avoid in debug sections.
>
> Regstrapped on x86_64-linux-gnu and i686-pc-linux-gnu, and manually
> cross-tested with both endiannesses (just to be sure ;-) with a
> mips64-elf target.  Ok to install?
>
> for  gcc/ChangeLog
>
>         PR debug/49366
>         * dwarf2out.c (loc_list_from_tree_1): Expand some CONSTRUCTORs
>         in DW_OP_pieces, just enough to handle pointers to member
>         functions.
>         (gen_remaining_tmpl_value_param_die_attribute): Use a location
>         expression on DWARFv5 if a constant value doesn't work.

This change broke early LTO debug by emitting locations during early finish.

A simplistic fix is to do

@@ -24410,7 +24794,8 @@ gen_remaining_tmpl_value_param_die_attri
          if (!tree_add_const_value_attribute (e->die, e->arg))
            {
              dw_loc_descr_ref loc = NULL;
-             if (dwarf_version >= 5 || !dwarf_strict)
+             if (! early_dwarf
+                 && (dwarf_version >= 5 || !dwarf_strict))
                loc = loc_descriptor_from_tree (e->arg, 2, NULL);
              if (loc)
                add_AT_loc (e->die, DW_AT_location, loc);

but in reality the issue is (again) that we can't refer to a DIE here.  Thus
it doesn't seem to be possible in dwarf to have debug info for

void func ();

  do <func> ();

with func being optimized out.

The issue with LTO / early debug is that we can't prune DIEs that refer
to optimized out entities from early debug output thus we have to create
those DIEs late only (in the above case, as tmpl_value_param_die_table
is not streamed, not at all for LTO).  Without LTO the issue is avoided
by resolve_addr pruning such references to optimized out entities from
location lists.

It would be nice to enhance dwarf to allow debug info for do <func>
so that printing the template parameter value is still possible even
if 'func' itself is optimized out.

Richard.

> for  gcc/testsuite/ChangeLog
>
>         PR debug/49366
>         * g++.dg/debug/dwarf2/template-params-12.H: New.
>         * g++.dg/debug/dwarf2/template-params-12f.C: New.
>         * g++.dg/debug/dwarf2/template-params-12g.C: New.
>         * g++.dg/debug/dwarf2/template-params-12n.C: New.
>         * g++.dg/debug/dwarf2/template-params-12s.C: New.
>         * g++.dg/debug/dwarf2/template-params-12u.C: New.
>         * g++.dg/debug/dwarf2/template-params-12v.C: New.
>         * g++.dg/debug/dwarf2/template-params-12w.C: New.
> ---
>  gcc/dwarf2out.c                                    | 93 +++++++++++++++++++++-
>  .../g++.dg/debug/dwarf2/template-params-12.H       | 17 ++++
>  .../g++.dg/debug/dwarf2/template-params-12f.C      |  7 ++
>  .../g++.dg/debug/dwarf2/template-params-12g.C      |  7 ++
>  .../g++.dg/debug/dwarf2/template-params-12n.C      | 10 +++
>  .../g++.dg/debug/dwarf2/template-params-12s.C      |  8 ++
>  .../g++.dg/debug/dwarf2/template-params-12u.C      |  7 ++
>  .../g++.dg/debug/dwarf2/template-params-12v.C      |  6 ++
>  .../g++.dg/debug/dwarf2/template-params-12w.C      |  6 ++
>  9 files changed, 160 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index e3cb586..c658220 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -16142,6 +16142,89 @@ loc_list_from_tree_1 (tree loc, int want_address,
>      case COMPLEX_CST:
>        if ((ret = cst_pool_loc_descr (loc)))
>         have_address = 1;
> +      else if (TREE_CODE (loc) == CONSTRUCTOR)
> +       {
> +         tree type = TREE_TYPE (loc);
> +         unsigned HOST_WIDE_INT size = int_size_in_bytes (type);
> +         unsigned HOST_WIDE_INT offset = 0;
> +         unsigned HOST_WIDE_INT cnt;
> +         constructor_elt *ce;
> +
> +         if (TREE_CODE (type) == RECORD_TYPE)
> +           {
> +             /* This is very limited, but it's enough to output
> +                pointers to member functions, as long as the
> +                referenced function is defined in the current
> +                translation unit.  */
> +             FOR_EACH_VEC_SAFE_ELT (CONSTRUCTOR_ELTS (loc), cnt, ce)
> +               {
> +                 tree val = ce->value;
> +
> +                 tree field = ce->index;
> +
> +                 if (val)
> +                   STRIP_NOPS (val);
> +
> +                 if (!field || DECL_BIT_FIELD (field))
> +                   {
> +                     expansion_failed (loc, NULL_RTX,
> +                                       "bitfield in record type constructor");
> +                     size = offset = (unsigned HOST_WIDE_INT)-1;
> +                     ret = NULL;
> +                     break;
> +                   }
> +
> +                 HOST_WIDE_INT fieldsize = tree_to_shwi (DECL_SIZE_UNIT (field));
> +                 unsigned HOST_WIDE_INT pos = int_byte_position (field);
> +                 gcc_assert (pos + fieldsize <= size);
> +                 if (pos < offset)
> +                   {
> +                     expansion_failed (loc, NULL_RTX,
> +                                       "out-of-order fields in record constructor");
> +                     size = offset = (unsigned HOST_WIDE_INT)-1;
> +                     ret = NULL;
> +                     break;
> +                   }
> +                 if (pos > offset)
> +                   {
> +                     ret1 = new_loc_descr (DW_OP_piece, pos - offset, 0);
> +                     add_loc_descr (&ret, ret1);
> +                     offset = pos;
> +                   }
> +                 if (val && fieldsize != 0)
> +                   {
> +                     ret1 = loc_descriptor_from_tree (val, want_address, context);
> +                     if (!ret1)
> +                       {
> +                         expansion_failed (loc, NULL_RTX,
> +                                           "unsupported expression in field");
> +                         size = offset = (unsigned HOST_WIDE_INT)-1;
> +                         ret = NULL;
> +                         break;
> +                       }
> +                     add_loc_descr (&ret, ret1);
> +                   }
> +                 if (fieldsize)
> +                   {
> +                     ret1 = new_loc_descr (DW_OP_piece, fieldsize, 0);
> +                     add_loc_descr (&ret, ret1);
> +                     offset = pos + fieldsize;
> +                   }
> +               }
> +
> +             if (offset != size)
> +               {
> +                 ret1 = new_loc_descr (DW_OP_piece, size - offset, 0);
> +                 add_loc_descr (&ret, ret1);
> +                 offset = size;
> +               }
> +
> +             have_address = !!want_address;
> +           }
> +         else
> +           expansion_failed (loc, NULL_RTX,
> +                             "constructor of non-record type");
> +       }
>        else
>        /* We can construct small constants here using int_loc_descriptor.  */
>         expansion_failed (loc, NULL_RTX,
> @@ -24177,7 +24260,15 @@ gen_remaining_tmpl_value_param_die_attribute (void)
>        FOR_EACH_VEC_ELT (*tmpl_value_parm_die_table, i, e)
>         {
>           if (!tree_add_const_value_attribute (e->die, e->arg))
> -           (*tmpl_value_parm_die_table)[j++] = *e;
> +           {
> +             dw_loc_descr_ref loc = NULL;
> +             if (dwarf_version >= 5 || !dwarf_strict)
> +               loc = loc_descriptor_from_tree (e->arg, 2, NULL);
> +             if (loc)
> +               add_AT_loc (e->die, DW_AT_location, loc);
> +             else
> +               (*tmpl_value_parm_die_table)[j++] = *e;
> +           }
>         }
>        tmpl_value_parm_die_table->truncate (j);
>      }
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
> new file mode 100644
> index 0000000..24c26e7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12.H
> @@ -0,0 +1,17 @@
> +// Tests for the fix for PR debug/49366.
> +struct B {
> +  void g();
> +  virtual void v() = 0;
> +  virtual void w();
> +};
> +void B::g() {}
> +void B::w() {}
> +struct S : B {
> +  void f();
> +  void v();
> +  void u();
> +};
> +void S::f() {}
> +void S::v() {}
> +template <void (B::*MF)()> void t() {}
> +template <void (S::*MF)()> void t() {}
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
> new file mode 100644
> index 0000000..e2da829
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12f.C
> @@ -0,0 +1,7 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]*\[^\n\]* DW_AT_location\n\[^\n\]* DW_OP_addr\n\[^\n\]*_ZN1S1fEv\[^\n\]*\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece\n\[^\n\]*\n\[^\n\]* DW_OP_lit0\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece" 1 } }
> +#include "template-params-12.H"
> +/* We get a location list with a pair of DW_OP_pieces for pointers to
> +   non-virtual member functions.  */
> +template void t<&S::f>();
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
> new file mode 100644
> index 0000000..813f71d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C
> @@ -0,0 +1,7 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]*\[^\n\]* DW_AT_location\n\[^\n\]* DW_OP_addr\n\[^\n\]*_ZN1B1gEv\[^\n\]*\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece\n\[^\n\]*\n\[^\n\]* DW_OP_lit0\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* DW_OP_piece" 1 } }
> +#include "template-params-12.H"
> +/* We get a location list with a pair of DW_OP_pieces for pointers to
> +   non-virtual member functions.  */
> +template void t<&S::g>();
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
> new file mode 100644
> index 0000000..d3c1f58
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
> @@ -0,0 +1,10 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
> +#include "template-params-12.H"
> +/* We get const_value for NULL pointers to member functions.  */
> +#if __cplusplus > 199711L // Ugh, C++98 barfs at both the cast and the overload.
> +template void t<static_cast<void (S::*)()>(0)>();
> +#else
> +template void t<&S::v>();
> +#endif
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
> new file mode 100644
> index 0000000..8940eaf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12s.C
> @@ -0,0 +1,8 @@
> +// { dg-options "-gdwarf-4 -gstrict-dwarf -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* \[^\n\]*DIE" 1 } }
> +#include "template-params-12.H"
> +/* We do NOT get a value or location for this one, because we've
> +   enabled strict DWARF 4, and it could only be emitted as a location,
> +   which is DWARF 5 only for template value params.  */
> +template void t<&S::f>();
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
> new file mode 100644
> index 0000000..f79fa84
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12u.C
> @@ -0,0 +1,7 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* \[^\n\]*DIE" 1 } }
> +#include "template-params-12.H"
> +/* We do NOT get a value or location for this one, because it would
> +   require a relocation to an undefined symbol in a debug section.  */
> +template void t<&S::u>();
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
> new file mode 100644
> index 0000000..11aabae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12v.C
> @@ -0,0 +1,6 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
> +#include "template-params-12.H"
> +/* We get const_value for pointers to virtual member functions.  */
> +template void t<&S::v>();
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C
> new file mode 100644
> index 0000000..11aabae
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12w.C
> @@ -0,0 +1,6 @@
> +// { dg-options "-gdwarf-2 -dA" }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
> +// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
> +#include "template-params-12.H"
> +/* We get const_value for pointers to virtual member functions.  */
> +template void t<&S::v>();
> --
> 2.1.0
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR49366] emit loc exprs for C++ non-virtual pmf template value parms
  2016-08-24 11:19 ` Richard Biener
@ 2016-08-30 23:15   ` Alexandre Oliva
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Oliva @ 2016-08-30 23:15 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Aug 24, 2016, Richard Biener <richard.guenther@gmail.com> wrote:

> The issue with LTO / early debug is that we can't prune DIEs that refer
> to optimized out entities from early debug output thus we have to create
> those DIEs late only (in the above case, as tmpl_value_param_die_table
> is not streamed, not at all for LTO).

I don't fully understand the problem you describe, but I wonder if the
infrastructure I wrote for the PR59319 patch, to revisit DIEs and prune
them if they reference other pruned DIEs, would help.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-30 23:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-23 15:25 [PR49366] emit loc exprs for C++ non-virtual pmf template value parms Alexandre Oliva
2016-08-02  3:08 ` Alexandre Oliva
2016-08-05 17:31 ` Jason Merrill
2016-08-05 19:38   ` Alexandre Oliva
2016-08-05 22:00   ` Alexandre Oliva
2016-08-24 11:19 ` Richard Biener
2016-08-30 23:15   ` Alexandre Oliva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).