Handle DW_AT_const_value/DW_FORM_strp variables

Handle DW_AT_const_value/DW_FORM_strp variables

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

Hi,

In some cases, a string variable can be optimized out by the
compiler, e.g., because it is not referenced anywhere in the code.
Debug info may still be generated for such cases, so printing
the variables contents should still work.  GDB is almost handling
that -- it's just the final glue is missing.  Here's a patch that adds
it, and a new testcase.

In C, it would look something like:

 const char a_string[] = "hello world!\n";

 int main ()
 {
 }

An unpatched GDB prints:

 (gdb) p a_string
 $1 = '\0' <repeats 13 times>

Patched prints:

 (gdb) p a_string
 $1 = "hello world!\n"

-- 
Pedro Alves

[-- Attachment #2: const_strp.diff --]
[-- Type: text/x-diff, Size: 8020 bytes --]

gdb/
2008-04-25  Pedro Alves  <pedro@codesourcery.com>

	* dwarf2read.c (dwarf2_const_value): Handle DW_FORM_strp.

gdb/testsuite/
2008-04-25  Pedro Alves  <pedro@codesourcery.com>

	* gdb.dwarf2/dw2-strp.S, gdb.dwarf2/dw2-strp.exp: New files.

---
 gdb/dwarf2read.c                      |    6 +
 gdb/testsuite/gdb.dwarf2/dw2-strp.S   |  142 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw2-strp.exp |   52 ++++++++++++
 3 files changed, 200 insertions(+)

Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2008-04-25 18:21:44.000000000 +0100
+++ src/gdb/dwarf2read.c	2008-04-25 19:04:03.000000000 +0100
@@ -7706,6 +7706,12 @@ dwarf2_const_value (struct attribute *at
 			      DW_ADDR (attr));
       SYMBOL_CLASS (sym) = LOC_CONST_BYTES;
       break;
+    case DW_FORM_strp:
+      /* DW_STRING is already allocated on the obstack, point directly
+	 to it.  */
+      SYMBOL_VALUE_BYTES (sym) = (gdb_byte *) DW_STRING (attr);
+      SYMBOL_CLASS (sym) = LOC_CONST_BYTES;
+      break;
     case DW_FORM_block1:
     case DW_FORM_block2:
     case DW_FORM_block4:
Index: src/gdb/testsuite/gdb.dwarf2/dw2-strp.S
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.dwarf2/dw2-strp.S	2008-04-25 18:22:38.000000000 +0100
@@ -0,0 +1,142 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 Free Software Foundation, Inc.
+
+   This program 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 of the License, or
+   (at your option) any later version.
+
+   This program 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 program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Debug information */
+
+	.section	.debug_info
+.Lcu1_begin:
+	.long		.Lcu1_end - .Lcu1_start	/* Length of Compilation Unit */
+.Lcu1_start:
+	.value		2			/* DWARF version number */
+	.long		.Ldebug_abbrev0		/* Offset Into Abbrev. Section */
+	.byte		4			/* Pointer Size (in bytes) */
+
+	/* CU die */
+	.uleb128	1			/* Abbrev: DW_TAG_compile_unit */
+	.long		.Lproducer		/* DW_AT_producer */
+	.byte		1			/* DW_AT_language (C) */
+
+.Larray_type:
+	.uleb128	2			/* Abbrev: DW_TAG_array_type */
+	.long		.Lchar_type-.Lcu1_begin	/* DW_AT_type */
+
+	.uleb128	3			/* Abbrev: DW_TAG_subrange_type */
+	.long		.Luint_type-.Lcu1_begin	/* DW_AT_type */
+	.byte		0xd			/* DW_AT_upper_bound */
+	.byte		0			/* End of children of die */
+
+.Luint_type:
+	.uleb128	4			/* Abbrev: DW_TAG_base_type */
+	.long		.Luint_str		/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		7			/* DW_AT_encoding */
+
+.Lchar_type:
+	.uleb128	4			/* Abbrev: DW_TAG_base_type */
+	.long		.Lchar_str		/* DW_AT_name */
+	.byte		1			/* DW_AT_byte_size */
+	.byte		6			/* DW_AT_encoding */
+
+	.uleb128	5			/* Abbrev: DW_TAG_variable */
+	.long		.Lvarname		/* DW_AT_name */
+	.long		.Lconst_type-.Lcu1_begin/* DW_AT_type */
+	.long		.Lvarcontents		/* DW_AT_const_value */
+	.byte		1			/* DW_AT_external */
+
+.Lconst_type:
+	.uleb128	6			/* Abbrev: DW_TAG_const_type */
+	.long		.Larray_type-.Lcu1_begin/* DW_AT_type */
+	.byte		0			/* End of children of CU */
+.Lcu1_end:
+
+	.section .debug_abbrev
+.Ldebug_abbrev0:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		0x1			/* has_children */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0xe			/* DW_FORM_strp */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x1			/* TAG: DW_TAG_array_type */
+	.byte		0x1			/* DW_children_yes */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x21			/* DW_TAG_subrange_type */
+	.byte		0x0			/* no children */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x2f			/* DW_AT_upper_bound */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0x0			/* no_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0xe			/* DW_FORM_strp */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	5			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0x0			/* no_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0xe			/* DW_FORM_strp */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x1c			/* DW_AT_const_value */
+	.uleb128	0xe			/* DW_FORM_strp */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	6			/* Abbrev code */
+	.uleb128	0x26			/* DW_TAG_const_type */
+	.byte		0x0			/* DW_children_no */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+/* String table */
+	.section .debug_str
+.Lproducer:
+	.string		"GNU C 3.3.3"
+.Lchar_str:
+	.string		"char"
+.Luint_str:
+	.string		"unsigned int"
+.Lvarname:
+	.string		"a_string"
+.Lvarcontents:
+	.string		"hello world!\n"
Index: src/gdb/testsuite/gdb.dwarf2/dw2-strp.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.dwarf2/dw2-strp.exp	2008-04-25 18:22:38.000000000 +0100
@@ -0,0 +1,52 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program 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 of the License, or
+# (at your option) any later version.
+#
+# This program 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 program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test printint string variables that are optimized out because they
+# are unreferenced in code, and whose contents appear in .debug_str.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0
+}
+
+set testfile "dw2-strp"
+set srcfile ${testfile}.S
+set binfile ${objdir}/${subdir}/${testfile}.x
+
+if  { [gdb_compile "${srcdir}/${subdir}/main.c" "main.o" object {debug}] != "" } {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${testfile}.o" object {nodebug}] != "" } {
+    return -1
+}
+
+if  { [gdb_compile "${testfile}.o main.o" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+gdb_test "p a_string" "\\\$1 = \"hello world!\\\\n\""
+gdb_test "ptype a_string" "type = char \\\[14\\\]"

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Joel Brobecker]

Hi Pedro,

> gdb/
> 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* dwarf2read.c (dwarf2_const_value): Handle DW_FORM_strp.
> 
> gdb/testsuite/
> 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* gdb.dwarf2/dw2-strp.S, gdb.dwarf2/dw2-strp.exp: New files.

Both patches look good to me. Please go ahead and commit.

> +# This test can only be run on targets which support DWARF-2 and use gas.
> +# For now pick a sampling of likely targets.
> +if {![istarget *-*-linux*]
> +    && ![istarget *-*-gnu*]
> +    && ![istarget *-*-elf*]
> +    && ![istarget *-*-openbsd*]
> +    && ![istarget arm-*-eabi*]
> +    && ![istarget powerpc-*-eabi*]} {
> +    return 0
> +}

I wonder if we should put that in a function somewhere. We have been
repeating this pretty large condition in several files, now. Just
thinking out loud...

-- 
Joel

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Pedro Alves]

A Friday 25 April 2008 19:28:58, Joel Brobecker escreveu:
> Hi Pedro,
>
> > gdb/
> > 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* dwarf2read.c (dwarf2_const_value): Handle DW_FORM_strp.
> >
> > gdb/testsuite/
> > 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* gdb.dwarf2/dw2-strp.S, gdb.dwarf2/dw2-strp.exp: New files.
>
> Both patches look good to me. Please go ahead and commit.
>

Thanks for the quick review.  It's in now.

> > +# This test can only be run on targets which support DWARF-2 and use
> > gas. +# For now pick a sampling of likely targets.
> > +if {![istarget *-*-linux*]
> > +    && ![istarget *-*-gnu*]
> > +    && ![istarget *-*-elf*]
> > +    && ![istarget *-*-openbsd*]
> > +    && ![istarget arm-*-eabi*]
> > +    && ![istarget powerpc-*-eabi*]} {
> > +    return 0
> > +}
>
> I wonder if we should put that in a function somewhere. We have been
> repeating this pretty large condition in several files, now. Just
> thinking out loud...

Indeed.

(
Thinking out load too:
However, we could also make the predicate the other way around.
Check for targets that *don't* support dwarf and *don't* use gas.  Could
they be fewer?  Probably some of the non-free Unixen, and older
xcoff/coff/a.out based targets.

E.g.  Windows/Cygwin aren't running these tests, and dwarf is a bit
more susceptible to bugs there in the sense that debug sections
don't get VMA == 0.
)

-- 
Pedro Alves

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Luis Machado]

Pedro,

This doesn't work as is on PPC. The ".value" pseudo-op it not available,
but using ".short" instead does the trick. Tested on x86 and PPC with
".short" for the DWARF version field.

Ok to change this?

Regards,
Luis


On Fri, 2008-04-25 at 19:59 +0100, Pedro Alves wrote:
> A Friday 25 April 2008 19:28:58, Joel Brobecker escreveu:
> > Hi Pedro,
> >
> > > gdb/
> > > 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> > >
> > > 	* dwarf2read.c (dwarf2_const_value): Handle DW_FORM_strp.
> > >
> > > gdb/testsuite/
> > > 2008-04-25  Pedro Alves  <pedro@codesourcery.com>
> > >
> > > 	* gdb.dwarf2/dw2-strp.S, gdb.dwarf2/dw2-strp.exp: New files.
> >
> > Both patches look good to me. Please go ahead and commit.
> >
> 
> Thanks for the quick review.  It's in now.
> 
> > > +# This test can only be run on targets which support DWARF-2 and use
> > > gas. +# For now pick a sampling of likely targets.
> > > +if {![istarget *-*-linux*]
> > > +    && ![istarget *-*-gnu*]
> > > +    && ![istarget *-*-elf*]
> > > +    && ![istarget *-*-openbsd*]
> > > +    && ![istarget arm-*-eabi*]
> > > +    && ![istarget powerpc-*-eabi*]} {
> > > +    return 0
> > > +}
> >
> > I wonder if we should put that in a function somewhere. We have been
> > repeating this pretty large condition in several files, now. Just
> > thinking out loud...
> 
> Indeed.
> 
> (
> Thinking out load too:
> However, we could also make the predicate the other way around.
> Check for targets that *don't* support dwarf and *don't* use gas.  Could
> they be fewer?  Probably some of the non-free Unixen, and older
> xcoff/coff/a.out based targets.
> 
> E.g.  Windows/Cygwin aren't running these tests, and dwarf is a bit
> more susceptible to bugs there in the sense that debug sections
> don't get VMA == 0.
> )

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

A Wednesday 30 April 2008 13:43:19, Luis Machado wrote:
> Pedro,
>
> This doesn't work as is on PPC. The ".value" pseudo-op it not available,
> but using ".short" instead does the trick. Tested on x86 and PPC with
> ".short" for the DWARF version field.
>

> Ok to change this?

Eh, oh well, sorry for that.  We should change the .long's to
explicit .4byte's and .value's to .2byte's as has been done to
the other dwarf tests.  Find replace is faster than another
mail roundtrip, so I went ahead.

Tested on x86_64-unknown-linux-gnu and x86-pc-linux-gnu.

OK ?

-- 
Pedro Alves

[-- Attachment #2: fix_const_strp.diff --]
[-- Type: text/x-diff, Size: 2560 bytes --]

2008-04-30  Pedro Alves  <pedro@codesourcery.com>

	* gdb.dwarf2/dw2-strp.S: Replace .long by .4byte and .value by
	.2byte.

---
 gdb/testsuite/gdb.dwarf2/dw2-strp.S |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Index: src/gdb/testsuite/gdb.dwarf2/dw2-strp.S
===================================================================
--- src.orig/gdb/testsuite/gdb.dwarf2/dw2-strp.S	2008-04-30 13:51:29.000000000 +0100
+++ src/gdb/testsuite/gdb.dwarf2/dw2-strp.S	2008-04-30 13:53:08.000000000 +0100
@@ -19,47 +19,47 @@
 
 	.section	.debug_info
 .Lcu1_begin:
-	.long		.Lcu1_end - .Lcu1_start	/* Length of Compilation Unit */
+	.4byte		.Lcu1_end - .Lcu1_start	/* Length of Compilation Unit */
 .Lcu1_start:
-	.value		2			/* DWARF version number */
-	.long		.Ldebug_abbrev0		/* Offset Into Abbrev. Section */
+	.2byte		2			/* DWARF version number */
+	.4byte		.Ldebug_abbrev0		/* Offset Into Abbrev. Section */
 	.byte		4			/* Pointer Size (in bytes) */
 
 	/* CU die */
 	.uleb128	1			/* Abbrev: DW_TAG_compile_unit */
-	.long		.Lproducer		/* DW_AT_producer */
+	.4byte		.Lproducer		/* DW_AT_producer */
 	.byte		1			/* DW_AT_language (C) */
 
 .Larray_type:
 	.uleb128	2			/* Abbrev: DW_TAG_array_type */
-	.long		.Lchar_type-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lchar_type-.Lcu1_begin	/* DW_AT_type */
 
 	.uleb128	3			/* Abbrev: DW_TAG_subrange_type */
-	.long		.Luint_type-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Luint_type-.Lcu1_begin	/* DW_AT_type */
 	.byte		0xd			/* DW_AT_upper_bound */
 	.byte		0			/* End of children of die */
 
 .Luint_type:
 	.uleb128	4			/* Abbrev: DW_TAG_base_type */
-	.long		.Luint_str		/* DW_AT_name */
+	.4byte		.Luint_str		/* DW_AT_name */
 	.byte		4			/* DW_AT_byte_size */
 	.byte		7			/* DW_AT_encoding */
 
 .Lchar_type:
 	.uleb128	4			/* Abbrev: DW_TAG_base_type */
-	.long		.Lchar_str		/* DW_AT_name */
+	.4byte		.Lchar_str		/* DW_AT_name */
 	.byte		1			/* DW_AT_byte_size */
 	.byte		6			/* DW_AT_encoding */
 
 	.uleb128	5			/* Abbrev: DW_TAG_variable */
-	.long		.Lvarname		/* DW_AT_name */
-	.long		.Lconst_type-.Lcu1_begin/* DW_AT_type */
-	.long		.Lvarcontents		/* DW_AT_const_value */
+	.4byte		.Lvarname		/* DW_AT_name */
+	.4byte		.Lconst_type-.Lcu1_begin/* DW_AT_type */
+	.4byte		.Lvarcontents		/* DW_AT_const_value */
 	.byte		1			/* DW_AT_external */
 
 .Lconst_type:
 	.uleb128	6			/* Abbrev: DW_TAG_const_type */
-	.long		.Larray_type-.Lcu1_begin/* DW_AT_type */
+	.4byte		.Larray_type-.Lcu1_begin/* DW_AT_type */
 	.byte		0			/* End of children of CU */
 .Lcu1_end:
 

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Daniel Jacobowitz]

On Wed, Apr 30, 2008 at 02:11:29PM +0100, Pedro Alves wrote:
> 2008-04-30  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* gdb.dwarf2/dw2-strp.S: Replace .long by .4byte and .value by
> 	.2byte.

OK.  Never heard of .value before.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Handle DW_AT_const_value/DW_FORM_strp variables

[Pedro Alves]

A Wednesday 30 April 2008 14:28:00, Daniel Jacobowitz wrote:
> On Wed, Apr 30, 2008 at 02:11:29PM +0100, Pedro Alves wrote:
> > 2008-04-30  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* gdb.dwarf2/dw2-strp.S: Replace .long by .4byte and .value by
> > 	.2byte.
>
> OK.  Never heard of .value before.

It's in.  Thanks.

-- 
Pedro Alves